Files
changedetection.io/changedetectionio/run_basic_tests.sh
T
dgtlmoonandClaude Opus 5 6318fc70f4 Browser fetchers - Report the real status code when Chrome aborts a bodiless error response (#4420)
* Browser fetchers - Report the real status code when Chrome aborts a bodiless error response

Chrome 153+ refuses to commit a navigation when a 4xx/5xx arrives with a zero-length body:
page.goto() raises net::ERR_HTTP_RESPONSE_CODE_FAILURE instead of returning the response. The
response is received fine, we just never get it as a return value, so the raw net:: string
landed in last_error instead of "Error - 404".

Verified against two browser images, same HTTP server:

  Chrome 153    empty-body 404 -> raises ERR_HTTP_RESPONSE_CODE_FAILURE
  Chrome 153    404 with body  -> status=404
  Chromium 119  empty-body 404 -> status=404
  Chromium 119  404 with body  -> status=404

The fix keeps the main-frame response from the 'response' event and hands that back when goto
raises, so .status / .all_headers() and the existing non-200 branch (which also captures the
screenshot) work unchanged. The latest matching response wins, so a redirect chain still
reports its final hop. Any other error re-raises as before, and if no response was captured we
re-raise too - the status is never invented, which keeps older browsers on exactly their old
path.

Two independent navigation sites needed it:

 - browser_steps.py action_goto_url - covers the playwright fetcher, the live Browser Steps UI,
   the Goto URL / Goto site steps, and the CloakBrowser plugin which imports it. This is the one
   that broke CI: content_fetchers/__init__.py forces playwright when a watch has browser steps,
   so test_non_200_errors_report_browsersteps ran the playwright path in the pyppeteer jobs too.

 - puppeteer.py - its own goto retry loop, used when FAST_PUPPETEER_CHROME_FETCHER is set and the
   watch has no browser steps. No test covers that path; verified by driving the fetcher directly.

Note pyppeteer exposes isNavigationRequest / frame / mainFrame as properties where playwright
uses is_navigation_request() as a method. Mixing them up raises 'bool' object is not callable,
which gets swallowed as a renderer page error rather than failing loudly. Checked against the
pinned pyppeteer-ng==2.0.0rc16.

Selenium is unaffected - it hardcodes status_code = 200 because WebDriver cannot see the HTTP
status, so it never reaches the non-200 branch.

Tested with the full CI browser set (test_content, test_errorhandling, test_fetch_data,
test_custom_js_before_content): 10 passed on each of Chrome 153 + playwright, Chrome 153 +
pyppeteer, Chromium 119 + playwright, Chromium 119 + pyppeteer, the last two against a canonical
Dockerfile.chromium119 build so Chromium is the only variable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* Fix unit test failure - install the navigation-response tracker only on a page that supports events

action_goto_url() registered its 'response' listener unconditionally, which broke
test_fetch_url_gate.py::TestBrowserStepGotoUrlGate::test_permitted_url_still_navigates:

    self.page.on("response", _keep_navigation_response)
    E  AttributeError: '_RecordingPage' object has no attribute 'on'

The three refusal tests in that class still passed because validate_fetch_url_async() raises before
reaching the listener, so only the permitted-URL case (the one that actually navigates) hit it.

The listener now lives in track_latest_navigation_response(), which returns None for a page that
has no event support instead of raising. That also removes a real inefficiency: registering per
navigation meant a page accumulated a listener per goto(), and 'response' fires for every
subresource - measured 133 events on getastra.com (54 script, 37 image, 19 fetch, 11 xhr, ...) of
which only 2 were navigations. The tracker is installed once per page and shared, verified as one
listener remaining after an install plus four navigations.

Tested: 494 unit + llm tests pass (was 1 failed / 480 passed), and the full browser set still
passes 10/10 on playwright and 10/10 on pyppeteer, so the Chrome 153 "Error - 404" recovery still
works through the shared tracker.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-12 13:15:44 +02:00

118 lines
4.1 KiB
Bash
Executable File

#!/bin/bash
# live_server will throw errors even with live_server_scope=function if I have the live_server setup in different functions
# and I like to restart the server for each test (and have the test cleanup after each test)
# merge request welcome :)
# 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
# Since theres no curl installed lets roll with python3
check_sanity() {
local port="$1"
if [ -z "$port" ]; then
echo "Usage: check_sanity <port>" >&2
return 1
fi
python3 - "$port" <<'PYCODE'
import sys, time, urllib.request, socket
port = sys.argv[1]
url = f'http://localhost:{port}'
ok = False
for _ in range(6): # --retry 6
try:
r = urllib.request.urlopen(url, timeout=3).read().decode()
if 'est-url-is-sanity' in r:
ok = True
break
except (urllib.error.URLError, ConnectionRefusedError, socket.error):
time.sleep(1)
sys.exit(0 if ok else 1)
PYCODE
}
data_sanity_test () {
# Restart data sanity test
cd ..
TMPDIR=$(mktemp -d)
PORT_N=$((5000 + RANDOM % (6501 - 5000)))
ALLOW_IANA_RESTRICTED_ADDRESSES=true ./changedetection.py -p $PORT_N -d $TMPDIR -u "https://localhost?test-url-is-sanity=1" &
PID=$!
sleep 5
kill $PID
sleep 2
ALLOW_IANA_RESTRICTED_ADDRESSES=true ./changedetection.py -p $PORT_N -d $TMPDIR &
PID=$!
sleep 5
# On a restart the URL should still be there
check_sanity $PORT_N || exit 1
kill $PID
cd $OLDPWD
# datastore looks alright, continue
}
data_sanity_test
echo "-------------------- Running rest of tests in parallel -------------------------------"
# REMOVE_REQUESTS_OLD_SCREENSHOTS disabled so that we can write a screenshot and send it in test_notifications.py without a real browser
FETCH_WORKERS=2 REMOVE_REQUESTS_OLD_SCREENSHOTS=false \
pytest tests/test_*.py \
-n 8 \
--dist=load \
-vvv \
-s \
--capture=no \
-k "not test_queue_system" \
--log-cli-level=DEBUG \
--log-cli-format="%(asctime)s [%(process)d] [%(levelname)s] %(name)s: %(message)s"
echo "---------------------------- DONE parallel test ---------------------------------------"
FETCH_WORKERS=20 pytest -vvv -s tests/test_queue_handler.py
echo "RUNNING WITH BASE_URL SET"
# Now re-run some tests with BASE_URL enabled
# Re #65 - Ability to include a link back to the installation, in the notification.
export BASE_URL="https://really-unique-domain.io"
# Re-run with HIDE_REFERER set - could affect login
export HIDE_REFERER=True
REMOVE_REQUESTS_OLD_SCREENSHOTS=false pytest -vv -s --maxfail=1 tests/test_notification.py tests/test_access_control.py
# Re #4309 - RSS pubDate/timestamps must be correct on containers that don't run UTC
# (Europe/Athens is UTC+2/+3, so any naive local->UTC relabelling shows up as a shifted date)
TZ=Europe/Athens pytest -vv -s --maxfail=1 tests/test_rss.py
# Re-run a few tests that will trigger brotli based storage
# And again with brotli+screenshot attachment
SNAPSHOT_BROTLI_COMPRESSION_THRESHOLD=5 REMOVE_REQUESTS_OLD_SCREENSHOTS=false pytest -vv -s --maxfail=1 --dist=load tests/test_backend.py tests/test_rss.py tests/test_unique_lines.py tests/test_notification.py tests/test_access_control.py
# Try high concurrency with aggressive worker restarts
FETCH_WORKERS=50 WORKER_MAX_RUNTIME=2 WORKER_MAX_JOBS=1 pytest tests/test_history_consistency.py -vv -l -s
# Check file:// will pickup a file when enabled
echo "Hello world" > /tmp/test-file.txt
ALLOW_FILE_URI=yes pytest -vv -s tests/test_security.py
# Run it again so that brotli kicks in
TEST_WITH_BROTLI=1 SNAPSHOT_BROTLI_COMPRESSION_THRESHOLD=100 FETCH_WORKERS=20 pytest tests/test_history_consistency.py -vv -l -s