Skip to content

Commit bb89a75

Browse files
sawenzelclaude
sawenzel
and
claude
committed
Run the O2DPG simulation tests against a CVMFS release
This replaces the aliBuild-based simulation-test check with a GitHub Actions job that runs the existing tests against a published O2PDPSuite release, and fixes two exit-code problems in the workflow tests. - test/run_tests.sh now owns the environment setup, sub-test selection and exit-code aggregation that lived in the alidist recipe body. - The new job runs on a self-hosted runner with CVMFS and compiles nothing. - run_workflow_tests.sh dropped ret_global_pwg from its final exit code, so a failed PWG workflow creation reported FAILED and still passed. - The AnalysisQC execution block was gated differently from the announcement above it; both now agree. - Three offline harnesses under test/tests/ check the entrypoint, the tag selection and the exit-code aggregation, and syntax-checks.yml runs them. - Touching test/needs-o2-dev opts a pull request into a source build against O2 dev, for changes that cannot use a published release. - The alidist recipe and the ali-bot check are left in place for now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 41b8e47 commit bb89a75

11 files changed

Lines changed: 603 additions & 17 deletions

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

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
---
2+
name: Simulation tests
3+
4+
'on':
5+
pull_request: {}
6+
workflow_dispatch:
7+
inputs:
8+
tag:
9+
description: 'O2PDPSuite tag to test against (default: newest daily)'
10+
type: string
11+
required: false
12+
13+
permissions: {}
14+
15+
concurrency:
16+
group: sim-tests-${{ github.event.pull_request.number || github.ref }}
17+
cancel-in-progress: true
18+
19+
jobs:
20+
sim-tests:
21+
name: Simulation tests against CVMFS
22+
runs-on: [self-hosted, cvmfs]
23+
timeout-minutes: 180
24+
25+
steps:
26+
- name: Checkout code
27+
uses: actions/checkout@v4
28+
with:
29+
# The changed-file logic diffs against the merge base, so the full
30+
# history is needed, not a shallow clone.
31+
fetch-depth: 0
32+
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 }}
39+
run: |
40+
set -eu
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"
43+
exit 1
44+
}
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"
47+
exit 1
48+
}
49+
merge_base=$(git merge-base "$BASE_SHA" "$HEAD_SHA") || {
50+
echo "::error title=Cannot compute diff base::git merge-base failed"
51+
exit 1
52+
}
53+
[ -n "$merge_base" ] || {
54+
echo "::error title=Cannot compute diff base::merge base is empty"
55+
exit 1
56+
}
57+
echo "sha=$merge_base" >> "$GITHUB_OUTPUT"
58+
59+
- name: Skip when not relevant or opted into the source build
60+
id: gate
61+
if: github.event_name == 'pull_request'
62+
env:
63+
BASE_SHA: ${{ steps.base.outputs.sha }}
64+
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
65+
run: |
66+
set -eu
67+
changed=$(git diff --name-only "$BASE_SHA" "$HEAD_SHA")
68+
# Opting in means *editing* the existing sentinel, so only count it
69+
# when it is Modified. A pull request that Adds it — which is what the
70+
# pull request introducing the sentinel does — would otherwise trip its
71+
# own opt-out and pass without testing anything.
72+
opted=$(git diff --name-only --diff-filter=M "$BASE_SHA" "$HEAD_SHA")
73+
if grep -qx 'test/needs-o2-dev' <<< "$opted" ; then
74+
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"
75+
echo "skip=true" >> "$GITHUB_OUTPUT"
76+
elif ! grep -qE '^(DATA/|MC/|test/|RelVal/)' <<< "$changed" ; then
77+
echo "::notice title=Skipped::no changed file matches DATA/, MC/, test/ or RelVal/"
78+
echo "skip=true" >> "$GITHUB_OUTPUT"
79+
else
80+
echo "skip=false" >> "$GITHUB_OUTPUT"
81+
fi
82+
83+
- name: Check the CVMFS environment
84+
if: steps.gate.outputs.skip != 'true'
85+
run: |
86+
set -eu
87+
test -d /cvmfs/alice.cern.ch || {
88+
echo "::error title=CVMFS unavailable::/cvmfs/alice.cern.ch is not mounted on this runner"
89+
exit 1
90+
}
91+
test -x /cvmfs/alice.cern.ch/bin/alienv || {
92+
echo "::error title=CVMFS unavailable::/cvmfs/alice.cern.ch/bin/alienv is missing"
93+
exit 1
94+
}
95+
96+
- name: Check the AliEn token
97+
if: steps.gate.outputs.skip != 'true'
98+
env:
99+
JALIEN_TOKEN_CERT: /run/alien-ci/tokencert.pem
100+
JALIEN_TOKEN_KEY: /run/alien-ci/tokenkey.pem
101+
run: |
102+
set -eu
103+
# DPL's CCDB backend refuses to talk to alice-ccdb.cern.ch without a
104+
# token and aborts the whole device, so check up front rather than
105+
# letting it surface as a confusing task crash deep in a workflow.
106+
for f in "$JALIEN_TOKEN_CERT" "$JALIEN_TOKEN_KEY" ; do
107+
test -r "$f" || {
108+
echo "::error title=No AliEn token::$f is missing or unreadable. On the runner host: systemctl start alien-ci-token.service"
109+
exit 1
110+
}
111+
done
112+
if ! openssl x509 -in "$JALIEN_TOKEN_CERT" -noout -checkend 3600 >/dev/null 2>&1 ; then
113+
echo "::error title=AliEn token expiring::the token expires within the hour. On the runner host: systemctl start alien-ci-token.service"
114+
exit 1
115+
fi
116+
echo "AliEn token valid until $(openssl x509 -in "$JALIEN_TOKEN_CERT" -noout -enddate | cut -d= -f2)"
117+
118+
- name: Resolve the O2PDPSuite tag
119+
id: tag
120+
if: steps.gate.outputs.skip != 'true'
121+
env:
122+
REQUESTED_TAG: ${{ inputs.tag }}
123+
PR_BODY: ${{ github.event.pull_request.body }}
124+
run: |
125+
set -eu
126+
# shellcheck source=test/ci/resolve_tag.sh
127+
. test/ci/resolve_tag.sh
128+
moduledir=/cvmfs/alice.cern.ch/el9-x86_64/Modules/modulefiles/O2PDPSuite
129+
requested=$REQUESTED_TAG
130+
if [ -z "$requested" ]; then
131+
# A PR can pin the release with a line "sim-tests-tag: <tag>".
132+
requested=$(printf '%s\n' "$PR_BODY" |
133+
sed -n 's/^[[:space:]]*sim-tests-tag:[[:space:]]*//p' | head -n 1 |
134+
tr -d '[:space:]')
135+
fi
136+
tag=$(resolve_o2pdpsuite_tag "$moduledir" "$requested") || {
137+
echo "::error title=No usable O2PDPSuite release::see the message above"
138+
exit 1
139+
}
140+
echo "Testing against O2PDPSuite::$tag"
141+
echo "tag=$tag" >> "$GITHUB_OUTPUT"
142+
143+
- name: Run the O2DPG tests
144+
if: steps.gate.outputs.skip != 'true'
145+
env:
146+
O2PDPSUITE_TAG: ${{ steps.tag.outputs.tag }}
147+
O2DPG_TEST_HASH_BASE: ${{ steps.base.outputs.sha }}
148+
O2DPG_TEST_HASH_HEAD: ${{ github.event.pull_request.head.sha }}
149+
JOBS: 8
150+
# o2dpg_sim_workflow.py calls JAlien(['whoami']) purely to fill the
151+
# AOD's --created-by field, and only when JALIEN_USER is unset. Setting
152+
# it keeps the tests from needing a GRID credential at all, which
153+
# matters here: anything readable by this account is readable by the
154+
# fork-pull-request code that runs as it.
155+
JALIEN_USER: alien-ci
156+
# DPL's CCDB backend needs a GRID token for alice-ccdb.cern.ch. These
157+
# are *paths* to a short-lived token minted from a service certificate
158+
# by a root-owned timer on the runner host; the certificate itself is
159+
# never readable by this account. Verified that paths work — the same
160+
# variables also accept PEM content, which is what ali-bot passes.
161+
JALIEN_TOKEN_CERT: /run/alien-ci/tokencert.pem
162+
JALIEN_TOKEN_KEY: /run/alien-ci/tokenkey.pem
163+
run: |
164+
set -eu
165+
# Everything after "-c" is joined into one string and re-evaluated
166+
# by the CVMFS alienv via "bash -c \"$*\"". Quoting here is applied
167+
# once then discarded, so it is safe for the runner's workspace path,
168+
# but a path with a space or "$" would break or double-evaluate.
169+
/cvmfs/alice.cern.ch/bin/alienv setenv "O2PDPSuite/$O2PDPSUITE_TAG" -c \
170+
env O2DPG_ROOT="$PWD" O2DPG_MC_CONFIG_ROOT="$PWD" \
171+
O2DPG_TEST_REPO_DIR="$PWD" \
172+
O2DPG_TEST_HASH_BASE="$O2DPG_TEST_HASH_BASE" \
173+
O2DPG_TEST_HASH_HEAD="$O2DPG_TEST_HASH_HEAD" \
174+
JOBS="$JOBS" \
175+
bash test/run_tests.sh
176+
177+
- name: Upload logs
178+
if: always()
179+
uses: actions/upload-artifact@v4
180+
with:
181+
name: o2dpg-test-logs
182+
path: |
183+
o2dpg_tests/**/*.log
184+
o2dpg_tests/**/*serverlog*
185+
o2dpg_tests/**/*workerlog*
186+
o2dpg_tests/**/*mergerlog*
187+
if-no-files-found: ignore
188+
retention-days: 14
189+
190+
- name: Prune test artifacts
191+
if: always()
192+
run: find o2dpg_tests -type f ! -name '*.log' ! -name '*serverlog*' ! -name '*workerlog*' ! -name '*mergerlog*' -delete || true

‎.github/workflows/syntax-checks.yml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,24 @@ jobs:
134134
- name: Run the FileIOGraph test suite
135135
run: python3 -m unittest discover -s UTILS/FileIOGraph/tests -t UTILS/FileIOGraph/tests
136136

137+
bash-harnesses:
138+
name: Test-harness unit tests
139+
runs-on: ubuntu-latest
140+
141+
steps:
142+
- name: Checkout code
143+
uses: actions/checkout@v4
144+
145+
- name: Run the offline test harnesses
146+
run: |
147+
error=0
148+
for t in test/tests/*.sh ; do
149+
echo "::group::$t"
150+
bash "$t" || error=1
151+
echo "::endgroup::"
152+
done
153+
exit "$error"
154+
137155
pylint:
138156
name: Pylint
139157
runs-on: ubuntu-latest

‎MC/bin/o2dpg_sim_workflow.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
# It aims to handle the different MC possible configurations
66
# It just creates a workflow.json txt file, to execute the workflow one must execute right after
77
# ${O2DPG_ROOT}/MC/bin/o2_dpg_workflow_runner.py -f workflow.json
8+
# The tests covering this script are described in test/README.md.
89
#
910
# Execution examples:
1011
# - pp PYTHIA jets, 2 events, triggered on high pT decay photons on all barrel calorimeters acceptance, eCMS 13 TeV

‎test/README.md‎

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ At the moment, the tests focus on generator configurations and custom generators
77

88
Tests are run via
99
```bash
10-
${O2DPG_ROOT}/test/run_tests.sh [--fail-immediately]
10+
${O2DPG_ROOT}/test/run_tests.sh [--fail-immediately] [--keep-artifacts] [SUBTEST...]
1111
```
1212

1313
Tests are run for changed
@@ -56,7 +56,32 @@ O2DPG_TEST_REPO_DIR=</path/to/source/O2DPG> ${O2DPG_ROOT}/test/run_tests.sh [--f
5656
```
5757
If you are inside the source directory, you can simply run
5858
```bash
59-
${O2DPG_ROOT}/test/run_tests.sh [--fail-immediately]
59+
${O2DPG_ROOT}/test/run_tests.sh [--fail-immediately] [--keep-artifacts] [SUBTEST...]
60+
```
61+
If the change you are testing is to a test script itself (`run_tests.sh` or
62+
any `run_*_tests.sh`), invoke the checkout's own entrypoint instead, e.g.
63+
`bash test/run_tests.sh` from inside the checkout: `run_tests.sh` finds its
64+
sub-scripts next to itself, so calling `${O2DPG_ROOT}/test/run_tests.sh`
65+
tests the *released* copy of the script you just edited, not your change,
66+
even with `O2DPG_TEST_REPO_DIR` pointed at the checkout.
67+
68+
### Running a subset
69+
70+
`run_tests.sh` runs the generator, workflow and RelVal sub-tests. To run only
71+
some of them, name them:
72+
73+
```bash
74+
${O2DPG_ROOT}/test/run_tests.sh generator relval
75+
```
76+
77+
The offline harnesses under `test/tests/` check the entrypoint's selection,
78+
exit-code aggregation and O2PDPSuite tag resolution without needing an O2
79+
environment:
80+
81+
```bash
82+
bash test/tests/run_tests_selection.sh
83+
bash test/tests/exit_code_aggregation.sh
84+
bash test/tests/resolve_tag.sh
6085
```
6186

6287
### Keeping all test artifacts
@@ -74,24 +99,33 @@ ${O2DPG_ROOT}/test/run_tests.sh -h
7499
```
75100
which will give you
76101
```
77-
usage: run_tests.sh [--fail-immediately] [--keep-artifacts]
102+
103+
usage: run_tests.sh [--fail-immediately] [--keep-artifacts] [SUBTEST...]
104+
105+
SUBTEST : one or more of: generator workflow relval (default: all)
78106
79107
FLAGS:
80108
81-
--fail-immediately : abort as soon as the first tests fails
82-
--keep-artifacts : keep simulation and tests artifacts, by default everything but the logs is removed after each test
109+
--fail-immediately : stop after the first failing sub-test
110+
--keep-artifacts : keep simulation artifacts, not just the logs
83111
84112
ENVIRONMENT VARIABLES:
85113
86-
O2DPG_TEST_REPO_DIR : Point to the source repository you want to test.
87-
O2DPG_TEST_HASH_BASE : The base hash you want to use for comparison (optional)
88-
O2DPG_TEST_HASH_HEAD : The head hash you want to use for comparison (optional)
89-
90-
If O2DPG_TEST_HASH_BASE is not set, it will be looked for ALIBUILD_BASE_HASH.
91-
If also not set, this will be set to HEAD~1. However, if there are unstaged
92-
changes, it will be set to HEAD.
114+
O2DPG_TEST_REPO_DIR : the source repository to test
115+
O2DPG_TEST_HASH_BASE : base hash for the changed-file diff (optional)
116+
O2DPG_TEST_HASH_HEAD : head hash for the changed-file diff (optional)
93117
94-
If O2DPG_TEST_HASH_HEAD is not set, it will be looked for ALIBUILD_HEAD_HASH.
95-
If also not set, this will be set to HEAD. However, if there are unstaged
96-
changes, it will left blank.
97118
```
119+
120+
## When your change needs an unreleased O2
121+
122+
The `Simulation tests against CVMFS` check runs against a published
123+
`O2PDPSuite` release, so a change that depends on an unmerged or unreleased O2
124+
commit cannot pass it. Two escape hatches, in order of preference:
125+
126+
1. If the O2 change is already in a published daily, pin it: add a line
127+
`sim-tests-tag: daily-YYYYMMDD-HHMM-1` to the pull request description.
128+
2. If it is not published anywhere yet, touch `test/needs-o2-dev` with a
129+
one-line reason and a link to the O2 pull request. That enables
130+
`build/O2DPG/sim/o2dev`, which builds O2 from source against `dev`. It is
131+
much slower, so it is opt-in.

‎test/ci/resolve_tag.sh‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#!/bin/bash
2+
3+
# Pick the O2PDPSuite release to test against. Kept free of CVMFS paths and of
4+
# CI variables so it can be exercised offline.
5+
6+
resolve_o2pdpsuite_tag()
7+
{
8+
local moduledir=${1:-}
9+
local requested=${2:-}
10+
11+
if [[ ! -d "${moduledir}" ]] ; then
12+
echo "resolve_o2pdpsuite_tag: no such directory: ${moduledir}" >&2
13+
return 1
14+
fi
15+
16+
if [[ -n "${requested}" ]] ; then
17+
case ${requested} in
18+
*[!A-Za-z0-9._-]* )
19+
echo "resolve_o2pdpsuite_tag: invalid tag: ${requested}" >&2
20+
return 1 ;;
21+
esac
22+
if [[ ! -e "${moduledir}/${requested}" ]] ; then
23+
echo "resolve_o2pdpsuite_tag: requested tag not available: ${requested}" >&2
24+
return 1
25+
fi
26+
echo "${requested}"
27+
return 0
28+
fi
29+
30+
local newest
31+
newest=$(find "${moduledir}" -maxdepth 1 -name 'daily-*' -printf '%f\n' 2>/dev/null |
32+
sort -V | tail -n 1)
33+
if [[ -z "${newest}" ]] ; then
34+
echo "resolve_o2pdpsuite_tag: no daily-* tag in ${moduledir}" >&2
35+
return 1
36+
fi
37+
echo "${newest}"
38+
}

‎test/needs-o2-dev‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Touch this file in a pull request that cannot be tested against a published
2+
O2PDPSuite release because it depends on an unreleased change in O2.
3+
Replace this text with a one-line reason and a link to the O2 pull request.
4+
Touching it enables the build/O2DPG/sim/o2dev check, which builds O2 from
5+
source against AliceO2Group/AliceO2 dev.

0 commit comments

Comments
 (0)