Skip to content

Commit 87c3ccf

Browse files
sawenzelclaude
andcommitted
Diff against the merge base and gate on relevance in sim-tests.yml
This fixes the sim-tests workflow diffing against the PR base branch tip instead of the merge base, and folds the path-based skip decision into the job so a required check cannot hang on a skipped run. - Added a step that resolves and verifies the merge base of the PR's base and head commits, failing loudly with an ::error:: annotation if either does not resolve. - The base-tip bug made every file changed on master since the PR branched look PR-authored, and every file deleted on master look newly added, which is exactly the false-attribution problem this migration exists to remove. - Removed the pull_request `paths:` filter and instead skip inside the gate step, printing a distinct ::notice:: for test/needs-o2-dev and for "no changed file under DATA/, MC/, test/, RelVal/". - Moved the gate step before the CVMFS check, so a PR that is skipped does not fail on a broken runner. - Pruned everything but logs from o2dpg_tests/ after the upload step. - Added a comment noting the alienv -c re-parsing hazard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent a38a0c5 commit 87c3ccf

1 file changed

Lines changed: 46 additions & 19 deletions

File tree

‎.github/workflows/sim-tests.yml‎

Lines changed: 46 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,7 @@
22
name: Simulation tests
33

44
'on':
5-
pull_request:
6-
paths:
7-
- 'DATA/**'
8-
- 'MC/**'
9-
- 'test/**'
10-
- 'RelVal/**'
5+
pull_request: {}
116
workflow_dispatch:
127
inputs:
138
tag:
@@ -31,40 +26,63 @@ jobs:
3126
- name: Checkout code
3227
uses: actions/checkout@v4
3328
with:
34-
# The changed-file logic diffs against the PR base, so the full
29+
# The changed-file logic diffs against the merge base, so the full
3530
# history is needed, not a shallow clone.
3631
fetch-depth: 0
3732

38-
- name: Check the CVMFS environment
33+
- name: Resolve the diff base
34+
id: base
35+
if: github.event_name == 'pull_request'
36+
env:
37+
BASE_SHA: ${{ github.event.pull_request.base.sha }}
38+
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
3939
run: |
4040
set -eu
41-
test -d /cvmfs/alice.cern.ch || {
42-
echo "::error title=CVMFS unavailable::/cvmfs/alice.cern.ch is not mounted on this runner"
41+
git rev-parse --verify "$BASE_SHA^{commit}" >/dev/null || {
42+
echo "::error title=Cannot resolve diff base::pull request base commit $BASE_SHA does not resolve in this checkout"
4343
exit 1
4444
}
45-
test -x /cvmfs/alice.cern.ch/bin/alienv || {
46-
echo "::error title=CVMFS unavailable::/cvmfs/alice.cern.ch/bin/alienv is missing"
45+
git rev-parse --verify "$HEAD_SHA^{commit}" >/dev/null || {
46+
echo "::error title=Cannot resolve diff head::pull request head commit $HEAD_SHA does not resolve in this checkout"
4747
exit 1
4848
}
49+
echo "sha=$(git merge-base "$BASE_SHA" "$HEAD_SHA")" >> "$GITHUB_OUTPUT"
4950
50-
- name: Skip when the pull request opted into the source build
51-
id: sentinel
51+
- name: Skip when not relevant or opted into the source build
52+
id: gate
5253
if: github.event_name == 'pull_request'
5354
env:
54-
BASE_SHA: ${{ github.event.pull_request.base.sha }}
55+
BASE_SHA: ${{ steps.base.outputs.sha }}
5556
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
5657
run: |
5758
set -eu
58-
if git diff --name-only "$BASE_SHA" "$HEAD_SHA" | grep -qx 'test/needs-o2-dev' ; then
59+
changed=$(git diff --diff-filter=AMR --name-only "$BASE_SHA" "$HEAD_SHA")
60+
if grep -qx 'test/needs-o2-dev' <<< "$changed" ; then
5961
echo "::notice title=Skipped::this pull request touches test/needs-o2-dev, so it is tested by build/O2DPG/sim/o2dev against O2 dev instead"
6062
echo "skip=true" >> "$GITHUB_OUTPUT"
63+
elif ! grep -qE '^(DATA/|MC/|test/|RelVal/)' <<< "$changed" ; then
64+
echo "::notice title=Skipped::no changed file matches DATA/, MC/, test/ or RelVal/"
65+
echo "skip=true" >> "$GITHUB_OUTPUT"
6166
else
6267
echo "skip=false" >> "$GITHUB_OUTPUT"
6368
fi
6469
70+
- name: Check the CVMFS environment
71+
if: steps.gate.outputs.skip != 'true'
72+
run: |
73+
set -eu
74+
test -d /cvmfs/alice.cern.ch || {
75+
echo "::error title=CVMFS unavailable::/cvmfs/alice.cern.ch is not mounted on this runner"
76+
exit 1
77+
}
78+
test -x /cvmfs/alice.cern.ch/bin/alienv || {
79+
echo "::error title=CVMFS unavailable::/cvmfs/alice.cern.ch/bin/alienv is missing"
80+
exit 1
81+
}
82+
6583
- name: Resolve the O2PDPSuite tag
6684
id: tag
67-
if: steps.sentinel.outputs.skip != 'true'
85+
if: steps.gate.outputs.skip != 'true'
6886
env:
6987
REQUESTED_TAG: ${{ inputs.tag }}
7088
PR_BODY: ${{ github.event.pull_request.body }}
@@ -88,14 +106,19 @@ jobs:
88106
echo "tag=$tag" >> "$GITHUB_OUTPUT"
89107
90108
- name: Run the O2DPG tests
91-
if: steps.sentinel.outputs.skip != 'true'
109+
if: steps.gate.outputs.skip != 'true'
92110
env:
93111
O2PDPSUITE_TAG: ${{ steps.tag.outputs.tag }}
94-
O2DPG_TEST_HASH_BASE: ${{ github.event.pull_request.base.sha }}
112+
O2DPG_TEST_HASH_BASE: ${{ steps.base.outputs.sha }}
95113
O2DPG_TEST_HASH_HEAD: ${{ github.event.pull_request.head.sha }}
96114
JOBS: 8
97115
run: |
98116
set -eu
117+
# Everything after "-c" is joined into one string and re-evaluated
118+
# by the CVMFS alienv via "bash -c \"$*\"", so this quoting is only
119+
# applied once here and then discarded. Safe for the runner's
120+
# actual workspace path, but a space or "$" in that path away from
121+
# breaking.
99122
/cvmfs/alice.cern.ch/bin/alienv setenv "O2PDPSuite/$O2PDPSUITE_TAG" -c \
100123
env O2DPG_ROOT="$PWD" O2DPG_MC_CONFIG_ROOT="$PWD" \
101124
O2DPG_TEST_REPO_DIR="$PWD" \
@@ -116,3 +139,7 @@ jobs:
116139
o2dpg_tests/**/*mergerlog*
117140
if-no-files-found: ignore
118141
retention-days: 14
142+
143+
- name: Prune test artifacts
144+
if: always()
145+
run: find o2dpg_tests -type f ! -name '*.log' ! -name '*serverlog*' ! -name '*workerlog*' ! -name '*mergerlog*' -delete || true

0 commit comments

Comments
 (0)