From 4a83dd9949407c58c9368549e11856a20101fd9c Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Sat, 29 Aug 2026 22:31:32 +0800 Subject: [PATCH 1/2] Make the CI unit test timeout fire before the job timeout does The test timeout exists so that a hanging test group is stopped while the job still has time to dump the container logs and upload them as artifacts. It never got that chance. The alarm is armed in the last step of the job, but the job budget starts at the first one, and checking out the repo, pulling the CI image and running the migration tests routinely take twenty minutes or more before any test runs. A fixed sixty minute test timeout could therefore only fire in a job GitHub had already cancelled, so every hang surfaced as a bare job timeout with no logs to explain it. --- .github/workflows/run-unit-tests.yml | 13 ++- .../testing/compute_remaining_test_timeout.py | 55 +++++++++++++ scripts/ci/testing/run_unit_tests.sh | 14 ++++ .../test_compute_remaining_test_timeout.py | 79 +++++++++++++++++++ .../tests/ci/testing/test_run_unit_tests.py | 38 +++++++++ 5 files changed, 197 insertions(+), 2 deletions(-) create mode 100644 scripts/ci/testing/compute_remaining_test_timeout.py create mode 100644 scripts/tests/ci/testing/test_compute_remaining_test_timeout.py create mode 100644 scripts/tests/ci/testing/test_run_unit_tests.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index a0a4e40242fb7..458ee33ca108f 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -129,11 +129,17 @@ on: # yamllint disable-line rule:truthy description: "The default branch of the repository" required: true type: string + job-timeout-minutes: + description: "How long the job may run before GitHub cancels it" + required: false + default: 65 + type: number permissions: contents: read jobs: tests: - timeout-minutes: 65 + # run_unit_tests.sh derives the tests' own timeout from what is left of this budget. + timeout-minutes: ${{ fromJSON(inputs.job-timeout-minutes) }} # yamllint disable rule:line-length name: "\ ${{ case(inputs.test-scope == 'Quarantined', 'Qrnt', inputs.test-scope == 'All', '', inputs.test-scope) }}\ @@ -176,9 +182,12 @@ jobs: AIRFLOW_MONITOR_DELAY_TIME_IN_SECONDS: "${{inputs.monitor-delay-time-in-seconds}}" VERBOSE: "true" DEFAULT_BRANCH: "${{ inputs.default-branch }}" - TOTAL_TEST_TIMEOUT: "3600" # 60 minutes in seconds + JOB_TIMEOUT_MINUTES: "${{ inputs.job-timeout-minutes }}" if: inputs.test-group == 'core' || inputs.skip-providers-tests != 'true' steps: + - name: "Record job start time" + shell: bash + run: echo "JOB_START_EPOCH=$(date +%s)" >> "${GITHUB_ENV}" - name: "Cleanup repo" shell: bash run: sudo rm -rf ${GITHUB_WORKSPACE}/* diff --git a/scripts/ci/testing/compute_remaining_test_timeout.py b/scripts/ci/testing/compute_remaining_test_timeout.py new file mode 100644 index 0000000000000..36e0266c1b0ff --- /dev/null +++ b/scripts/ci/testing/compute_remaining_test_timeout.py @@ -0,0 +1,55 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import argparse +import time + +# Reserved so the test timeout handler can stop the containers and dump their logs, and the job +# still has time to upload those logs as artifacts, before GitHub cancels the job. +TEARDOWN_GRACE_SECONDS = 5 * 60 + +# Floor applied when the steps before the tests already ate the budget - the job is doomed either +# way, but the tests still get a chance to report something rather than none at all. +MINIMUM_TEST_TIMEOUT_SECONDS = 10 * 60 + + +def compute_remaining_test_timeout(job_timeout_seconds: int, elapsed_seconds: int) -> int: + """Return how long tests may run so their own timeout fires before the job timeout does.""" + remaining_seconds = job_timeout_seconds - elapsed_seconds - TEARDOWN_GRACE_SECONDS + return max(remaining_seconds, MINIMUM_TEST_TIMEOUT_SECONDS) + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Print the number of seconds the tests may run within the remaining job budget." + ) + parser.add_argument( + "--job-timeout-minutes", type=int, required=True, help="The job's `timeout-minutes` value" + ) + parser.add_argument( + "--job-start-epoch", type=int, required=True, help="Unix timestamp stamped when the job started" + ) + args = parser.parse_args() + # Wall clock rather than time.monotonic(): the start timestamp comes from an earlier step in a + # different process, so only an absolute clock is comparable across the two. + elapsed_seconds = int(time.time()) - args.job_start_epoch + print(compute_remaining_test_timeout(args.job_timeout_minutes * 60, elapsed_seconds)) + + +if __name__ == "__main__": + main() diff --git a/scripts/ci/testing/run_unit_tests.sh b/scripts/ci/testing/run_unit_tests.sh index 0d6fc76bbda3c..786b40f4decb8 100755 --- a/scripts/ci/testing/run_unit_tests.sh +++ b/scripts/ci/testing/run_unit_tests.sh @@ -30,6 +30,20 @@ fi TEST_GROUP=${1} TEST_SCOPE=${2} +# The whole job - not only this step - has to fit in JOB_TIMEOUT_MINUTES, so the tests get whatever +# is left of that budget rather than a fixed value. +if [[ -n "${JOB_START_EPOCH:-}" && -n "${JOB_TIMEOUT_MINUTES:-}" ]]; then + TOTAL_TEST_TIMEOUT=$(python "$(dirname "${BASH_SOURCE[0]}")/compute_remaining_test_timeout.py" \ + --job-timeout-minutes "${JOB_TIMEOUT_MINUTES}" --job-start-epoch "${JOB_START_EPOCH}") || exit 1 + export TOTAL_TEST_TIMEOUT + echo "${COLOR_BLUE}Tests get ${TOTAL_TEST_TIMEOUT}s of the ${JOB_TIMEOUT_MINUTES}m job budget${COLOR_RESET}" +elif [[ "${GITHUB_ACTIONS:-}" == "true" ]]; then + # Letting breeze fall back to its own default here would restore the timeout that cannot + # fire, and nothing would say so - the job would stay green until something hung. + echo "${COLOR_RED}JOB_START_EPOCH and JOB_TIMEOUT_MINUTES must both be set in CI${COLOR_RESET}" + exit 1 +fi + function core_tests() { echo "${COLOR_BLUE}Running core tests${COLOR_RESET}" set +e diff --git a/scripts/tests/ci/testing/test_compute_remaining_test_timeout.py b/scripts/tests/ci/testing/test_compute_remaining_test_timeout.py new file mode 100644 index 0000000000000..ff9ef7a4ffafe --- /dev/null +++ b/scripts/tests/ci/testing/test_compute_remaining_test_timeout.py @@ -0,0 +1,79 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import importlib.util +import sys +from datetime import datetime, timezone +from pathlib import Path + +import pytest +import time_machine + +MODULE_PATH = ( + Path(__file__).resolve().parents[4] / "scripts" / "ci" / "testing" / "compute_remaining_test_timeout.py" +) + + +@pytest.fixture +def timeout_module(): + module_name = "test_compute_remaining_test_timeout_module" + sys.modules.pop(module_name, None) + spec = importlib.util.spec_from_file_location(module_name, MODULE_PATH) + module = importlib.util.module_from_spec(spec) + assert spec.loader is not None + sys.modules[module_name] = module + spec.loader.exec_module(module) + return module + + +@pytest.mark.parametrize( + ("job_timeout_seconds", "elapsed_seconds", "expected_seconds"), + [ + pytest.param(65 * 60, 8 * 60, 3120, id="image-pull-only"), + pytest.param(65 * 60, 25 * 60, 2100, id="image-pull-and-slow-migration-tests"), + pytest.param(65 * 60, 55 * 60, 600, id="floor-applies-when-budget-nearly-gone"), + pytest.param(65 * 60, 70 * 60, 600, id="floor-applies-when-budget-overrun"), + ], +) +def test_compute_remaining_test_timeout( + timeout_module, job_timeout_seconds, elapsed_seconds, expected_seconds +): + assert ( + timeout_module.compute_remaining_test_timeout(job_timeout_seconds, elapsed_seconds) + == expected_seconds + ) + + +@time_machine.travel("2026-08-29 12:10:00+00:00", tick=False) +def test_main_derives_the_budget_left_from_the_job_start_time(timeout_module, monkeypatch, capsys): + job_start_epoch = int(datetime(2026, 8, 29, 12, 0, tzinfo=timezone.utc).timestamp()) + monkeypatch.setattr( + sys, + "argv", + [ + "compute_remaining_test_timeout.py", + "--job-timeout-minutes", + "65", + "--job-start-epoch", + str(job_start_epoch), + ], + ) + + timeout_module.main() + + assert int(capsys.readouterr().out) == (65 - 10 - 5) * 60 diff --git a/scripts/tests/ci/testing/test_run_unit_tests.py b/scripts/tests/ci/testing/test_run_unit_tests.py new file mode 100644 index 0000000000000..ecd6204c63440 --- /dev/null +++ b/scripts/tests/ci/testing/test_run_unit_tests.py @@ -0,0 +1,38 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import subprocess +from pathlib import Path + +SCRIPT_PATH = Path(__file__).resolve().parents[4] / "scripts" / "ci" / "testing" / "run_unit_tests.sh" + +# Deliberately excludes the directory holding breeze, so a regression that lets the script run on +# past the guard cannot start a real test run - it fails to find breeze instead. +MINIMAL_ENVIRONMENT = {"PATH": "/usr/bin:/bin"} + + +def test_run_unit_tests_aborts_when_the_job_budget_is_missing_in_github_actions(): + result = subprocess.run( + ["bash", str(SCRIPT_PATH), "core", "DB"], + env={**MINIMAL_ENVIRONMENT, "GITHUB_ACTIONS": "true"}, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 1 + assert "JOB_START_EPOCH and JOB_TIMEOUT_MINUTES must both be set" in result.stdout From afe13c274a58cd17200316cfde320717e5b3864b Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Mon, 31 Aug 2026 00:38:14 +0800 Subject: [PATCH 2/2] Print the CI test budget in minutes and seconds The line reported the budget left in seconds next to a job timeout in minutes, so anyone reading it had to convert one to compare them. --- scripts/ci/testing/run_unit_tests.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/ci/testing/run_unit_tests.sh b/scripts/ci/testing/run_unit_tests.sh index 786b40f4decb8..cc0fe6c849edd 100755 --- a/scripts/ci/testing/run_unit_tests.sh +++ b/scripts/ci/testing/run_unit_tests.sh @@ -36,7 +36,8 @@ if [[ -n "${JOB_START_EPOCH:-}" && -n "${JOB_TIMEOUT_MINUTES:-}" ]]; then TOTAL_TEST_TIMEOUT=$(python "$(dirname "${BASH_SOURCE[0]}")/compute_remaining_test_timeout.py" \ --job-timeout-minutes "${JOB_TIMEOUT_MINUTES}" --job-start-epoch "${JOB_START_EPOCH}") || exit 1 export TOTAL_TEST_TIMEOUT - echo "${COLOR_BLUE}Tests get ${TOTAL_TEST_TIMEOUT}s of the ${JOB_TIMEOUT_MINUTES}m job budget${COLOR_RESET}" + TIMEOUT_DISPLAY="$((TOTAL_TEST_TIMEOUT / 60))m$((TOTAL_TEST_TIMEOUT % 60))s" + echo "${COLOR_BLUE}Tests get ${TIMEOUT_DISPLAY} of the ${JOB_TIMEOUT_MINUTES}m job budget${COLOR_RESET}" elif [[ "${GITHUB_ACTIONS:-}" == "true" ]]; then # Letting breeze fall back to its own default here would restore the timeout that cannot # fire, and nothing would say so - the job would stay green until something hung.