From 52a856d6e69969475d40eea19b976b1bbc0009f4 Mon Sep 17 00:00:00 2001 From: dgtlmoon Date: Sat, 12 Sep 2026 10:54:09 +0200 Subject: [PATCH] CI - Make a fail-fast test abort say what was skipped rather than looking like a total failure Fail-fast is kept deliberately - the first failure is nearly always the real problem and it keeps the run short - but nothing said so, which made a single failing assertion read as "every browser test is broken". The playwright and pyppeteer jobs each ran four pytest files as four commands in one `run:` block, which GitHub executes under `bash -e`. tests/visualselector/test_fetch_data.py is the third, so when one 404 assertion failed there, test_custom_js_before_content.py never ran, and the later "Headers and requests" and "Restock detection" steps were skipped as a consequence - three test files silently dropped, reported only as dashes in the job list. run_basic_tests.sh has the same shape: 8 independent pytest groups under `set -e`, so a failure in the first parallel group hides the 7 after it. No behaviour change to when we stop - only to what gets reported: - Each browser test file now runs inside its own ::group:: so the log is navigable, and the failing file is named in a ::error:: annotation that states plainly that the remaining files and steps were SKIPPED, not failed. - run_basic_tests.sh gets an ERR trap saying the same thing, with the line number of the group that aborted. Verified the loop stops on the third file, names it and exits 1, that the all-pass path still exits 0, and that the trap reports the failing line while preserving the exit code. YAML parses and run_basic_tests.sh passes bash -n. Co-Authored-By: Claude Opus 5 (1M context) --- .../test-stack-reusable-workflow.yml | 33 ++++++++++++++----- changedetectionio/run_basic_tests.sh | 6 ++++ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test-stack-reusable-workflow.yml b/.github/workflows/test-stack-reusable-workflow.yml index 0fe623875..dae9e9dd7 100644 --- a/.github/workflows/test-stack-reusable-workflow.yml +++ b/.github/workflows/test-stack-reusable-workflow.yml @@ -201,10 +201,20 @@ jobs: - name: Playwright - Specific tests in built container run: | - docker run --rm -e "FLASK_SERVER_NAME=cdio" -e "PLAYWRIGHT_DRIVER_URL=ws://sockpuppetbrowser:3000" --network changedet-network --hostname=cdio test-changedetectionio bash -c 'cd changedetectionio;pytest -vv --capture=tee-sys --showlocals --tb=long --live-server-host=0.0.0.0 --live-server-port=5004 tests/fetchers/test_content.py' - docker run --rm -e "FLASK_SERVER_NAME=cdio" -e "PLAYWRIGHT_DRIVER_URL=ws://sockpuppetbrowser:3000" --network changedet-network --hostname=cdio test-changedetectionio bash -c 'cd changedetectionio;pytest -vv --capture=tee-sys --showlocals --tb=long --live-server-host=0.0.0.0 --live-server-port=5004 tests/test_errorhandling.py' - docker run --rm -e "FLASK_SERVER_NAME=cdio" -e "PLAYWRIGHT_DRIVER_URL=ws://sockpuppetbrowser:3000" --network changedet-network --hostname=cdio test-changedetectionio bash -c 'cd changedetectionio;pytest -vv --capture=tee-sys --showlocals --tb=long --live-server-host=0.0.0.0 --live-server-port=5004 tests/visualselector/test_fetch_data.py' - docker run --rm -e "FLASK_SERVER_NAME=cdio" -e "PLAYWRIGHT_DRIVER_URL=ws://sockpuppetbrowser:3000" --network changedet-network --hostname=cdio test-changedetectionio bash -c 'cd changedetectionio;pytest -vv --capture=tee-sys --showlocals --tb=long --live-server-host=0.0.0.0 --live-server-port=5004 tests/fetchers/test_custom_js_before_content.py' + # Deliberately fail fast - the first failure is nearly always the real problem and it + # keeps the run short. Each file is wrapped in its own log group and the failing one is + # named explicitly, because everything after it is SKIPPED rather than run, and that is + # otherwise easy to misread as "the whole browser suite broke". + for t in tests/fetchers/test_content.py tests/test_errorhandling.py tests/visualselector/test_fetch_data.py tests/fetchers/test_custom_js_before_content.py; do + echo "::group::pytest $t" + if ! docker run --rm -e "FLASK_SERVER_NAME=cdio" -e "PLAYWRIGHT_DRIVER_URL=ws://sockpuppetbrowser:3000" --network changedet-network --hostname=cdio test-changedetectionio \ + bash -c "cd changedetectionio;pytest -vv --capture=tee-sys --showlocals --tb=long --live-server-host=0.0.0.0 --live-server-port=5004 $t"; then + echo "::endgroup::" + echo "::error::$t FAILED - stopping here. Any later test file and the remaining steps of this job were SKIPPED, not failed." + exit 1 + fi + echo "::endgroup::" + done - name: Playwright - Headers and requests run: | @@ -242,10 +252,17 @@ jobs: - name: Pyppeteer - Specific tests in built container run: | - docker run --rm -e "FLASK_SERVER_NAME=cdio" -e "FAST_PUPPETEER_CHROME_FETCHER=True" -e "PLAYWRIGHT_DRIVER_URL=ws://sockpuppetbrowser:3000" --network changedet-network --hostname=cdio test-changedetectionio bash -c 'cd changedetectionio;pytest --live-server-host=0.0.0.0 --live-server-port=5004 tests/fetchers/test_content.py' - docker run --rm -e "FLASK_SERVER_NAME=cdio" -e "FAST_PUPPETEER_CHROME_FETCHER=True" -e "PLAYWRIGHT_DRIVER_URL=ws://sockpuppetbrowser:3000" --network changedet-network --hostname=cdio test-changedetectionio bash -c 'cd changedetectionio;pytest --live-server-host=0.0.0.0 --live-server-port=5004 tests/test_errorhandling.py' - docker run --rm -e "FLASK_SERVER_NAME=cdio" -e "FAST_PUPPETEER_CHROME_FETCHER=True" -e "PLAYWRIGHT_DRIVER_URL=ws://sockpuppetbrowser:3000" --network changedet-network --hostname=cdio test-changedetectionio bash -c 'cd changedetectionio;pytest --live-server-host=0.0.0.0 --live-server-port=5004 tests/visualselector/test_fetch_data.py' - docker run --rm -e "FLASK_SERVER_NAME=cdio" -e "FAST_PUPPETEER_CHROME_FETCHER=True" -e "PLAYWRIGHT_DRIVER_URL=ws://sockpuppetbrowser:3000" --network changedet-network --hostname=cdio test-changedetectionio bash -c 'cd changedetectionio;pytest --live-server-host=0.0.0.0 --live-server-port=5004 tests/fetchers/test_custom_js_before_content.py' + # Fail fast, but name the file that failed - see the note in the playwright job above + for t in tests/fetchers/test_content.py tests/test_errorhandling.py tests/visualselector/test_fetch_data.py tests/fetchers/test_custom_js_before_content.py; do + echo "::group::pytest $t" + if ! docker run --rm -e "FLASK_SERVER_NAME=cdio" -e "FAST_PUPPETEER_CHROME_FETCHER=True" -e "PLAYWRIGHT_DRIVER_URL=ws://sockpuppetbrowser:3000" --network changedet-network --hostname=cdio test-changedetectionio \ + bash -c "cd changedetectionio;pytest --live-server-host=0.0.0.0 --live-server-port=5004 $t"; then + echo "::endgroup::" + echo "::error::$t FAILED - stopping here. Any later test file and the remaining steps of this job were SKIPPED, not failed." + exit 1 + fi + echo "::endgroup::" + done - name: Pyppeteer - Headers and requests checks run: | diff --git a/changedetectionio/run_basic_tests.sh b/changedetectionio/run_basic_tests.sh index f69ef75d2..f7e6f380d 100755 --- a/changedetectionio/run_basic_tests.sh +++ b/changedetectionio/run_basic_tests.sh @@ -9,6 +9,12 @@ # exit when any command fails set -e +# Failing fast is deliberate here (the first failure is usually the real problem, and it keeps +# the run short) - but this script runs 8 independent pytest groups, so make it obvious that the +# groups after the failure were SKIPPED rather than passed. Otherwise one failing test reads as +# "the whole basic suite is broken". +trap 'rc=$?; echo "::error::run_basic_tests.sh aborted at line $LINENO (exit $rc) - the test groups after this point were SKIPPED, not run"' ERR + SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) rm tests/logs/* -f