Skip to content

Harden benchmark script against pull request changes #185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
richardlau merged 3 commits into from
Apr 8, 2025
+29 −5
Merged

Harden benchmark script against pull request changes #185

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27e498e
Harden benchmark script against pull request changes
richardlau Apr 3, 2025
b6a1f2d
Use shallow clone in benchmark script
richardlau Apr 3, 2025
843fc40
Rename BRANCH parameter to BASE
richardlau Apr 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions benchmarks/community-benchmark/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,23 @@
#!/bin/bash
function assertShaLike() {
local shaVarName="${1}"
local sha="${!1}"
if [ -z "${sha}" ] || ! echo "${sha}" | grep -qE '^[0-9a-fA-F]+$'; then
echo "${shaVarName} (${sha}) does not look like a commit SHA"
exit
fi
}

function assertMatchesPullRequest() {
local shaVarName="${1}"
local sha="${!1}"
local pr_id="${!2}"
if [[ ! "$(git ls-remote origin refs/pull/${pr_id}/head)" =~ ^${sha}.*refs/pull/${pr_id}/head$ ]]; then
echo "${shaVarName} (${sha}) does not match HEAD for pull request ${pr_id}"
exit
fi
}

function mandatory() {
if [ -z "${!1}" ]; then
echo "${1} not set"
Expand All @@ -25,8 +44,9 @@ echo "Use case 1: We want to test the impact of a PR on a branch."
echo "To run this, declare:"
echo "The script expects the following variables to be set:"
echo "CATEGORY = a category of tests to run - folders in benchmark/"
echo "BRANCH = the branch the test should be based off. e.g. master"
echo "BASE = the branch the test should be based off. e.g. master"
echo "PULL_ID = the pull request that contains changes to test"
echo "TARGET = the SHA of the commit HEAD that contains changes to test"
echo "-------------------------------------------------------------"
echo "Use case 2: We want to compare two branches, tags or commits."
echo "To run this, declare:"
Expand All @@ -48,8 +68,10 @@ if [ -z $PULL_ID ]; then
mandatory TARGET
else
export USE_CASE=1
mandatory BRANCH
mandatory BASE
mandatory PULL_ID
mandatory TARGET
assertShaLike TARGET
fi
mandatory CATEGORY
optional RUNS
Expand All @@ -59,11 +81,13 @@ getMACHINE_THREADS=`cat /proc/cpuinfo |grep processor|tail -n1|awk {'print $3'}`
let getMACHINE_THREADS=getMACHINE_THREADS+1 #getting threads this way is 0 based. Add one
optional MACHINE_THREADS $getMACHINE_THREADS
rm -rf node
git clone https://github.com/nodejs/node.git
git clone --depth=1 https://github.com/nodejs/node.git
Copy link

@aduh95 aduh95 Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
git clone --depth=1 https://github.com/nodejs/node.git
git clone --depth=1 --branch "$BASE" https://github.com/nodejs/node.git

Copy link
Member Author

@richardlau richardlau Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave this for a follow up. Note that I think the suggestion will also require adding a fetch for TARGET in use case 2 to allow git checkout TARGET to succeed when TARGET points to a different branch or commit not on BASE.

cd node
case $USE_CASE in
1)
git checkout $BRANCH
# Validate TARGET and pull request HEAD are consistent
assertMatchesPullRequest TARGET PULL_ID
git checkout $BASE
;;
2)
git checkout $BASE
Expand All @@ -81,7 +105,7 @@ mv out/Release/node ./node-master
# build pr
case $USE_CASE in
1)
curl -L https://github.com/nodejs/node/pull/${PULL_ID}.patch|git apply -3
curl -L "https://github.com/nodejs/node/compare/$(git rev-parse HEAD)...${TARGET}.patch"|git apply -3
;;
2)
git checkout $TARGET
Expand Down