diff --git a/.github/workflows/experimental.yml b/.github/workflows/experimental.yml index 0b8c44a352..4ea4880c17 100644 --- a/.github/workflows/experimental.yml +++ b/.github/workflows/experimental.yml @@ -489,7 +489,11 @@ jobs: # As `test`'s, under a name of its own: ci-timing.yml reads `test`'s # `junit-unit-*` files alone, and this one is the record of GraalPy's - # runtime. The glob takes each shard's JUnit file too (#1963). + # runtime. The glob takes each shard's JUnit file too (#1963). #2073: a + # run the watcher stopped as superseded has pytest killed before it + # writes junit.xml, and its `Superseded` notice already says why, so the + # watcher's `stopped=true` turns the missing-file warning off; a run that + # ran to its end and wrote no file still warns. - name: Upload test timings if: ${{ !cancelled() && steps.install.outcome == 'success' }} continue-on-error: true @@ -497,6 +501,7 @@ jobs: with: name: junit-experimental-${{ matrix.python-version }} path: ${{ runner.temp }}/junit*.xml + if-no-files-found: ${{ steps.tests.outputs.stopped == 'true' && 'ignore' || 'warn' }} retention-days: 14 # #1740: what `tests-status` and the monitor's last sample say about the diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index 8459421e37..87ac057bcd 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -892,7 +892,11 @@ in it, since they run as the runner's user under a root-owned ``sudo`` and in ``/etc/sudoers`` sets ``use_pty``, which sudoers(5) says has no effect where ``sudo`` has no terminal, as in a step; the scope is reached by name either way. Once the command exits, the watcher writes a ``Superseded`` notice and a job-summary line and -exits 0, so the leg ends green and frees its runner. Every wait is bounded: where the +exits 0, so the leg ends green and frees its runner. It also writes ``stopped=true`` +to the step's outputs: pytest, stopped, writes no ``junit.xml``, so ``Upload test +timings`` then sets ``if-no-files-found: ignore`` rather than warn of the missing +file the notice already explains (`#2073 +`__). Every wait is bounded: where the command outlives SIGKILL by 15 seconds, it writes a ``Superseded run not stopped`` warning and exits 70, not 0, since the tests may run on, holding the step's output, until ``timeout`` or the step's cap ends them, and the failed step is then a warning diff --git a/tests/project/test_superseded_junit_upload_2073.py b/tests/project/test_superseded_junit_upload_2073.py new file mode 100644 index 0000000000..d451a7ed2f --- /dev/null +++ b/tests/project/test_superseded_junit_upload_2073.py @@ -0,0 +1,160 @@ +# -*- coding: utf-8 -*- +"""A superseded Experimental leg warns of no missing JUnit file (#2073). + +In Experimental Tests run 39898029852 both legs were stopped by +util/superseded.py's ``watch`` 60 s in, ``main`` having moved on, and each then +warned ``No files were found with the provided path: +/home/runner/work/_temp/junit*.xml``: pytest, stopped, writes no JUnit file, +and ``Upload test timings`` ran on as for any finished run. The watcher's +``Superseded`` notice already says why the file is missing, so the watcher +now writes ``stopped=true`` to the step's outputs and the upload reads it. + +""" + +from __future__ import annotations + +import io +import os +import pathlib +import re +import shutil +import sys +import tempfile +import threading +import unittest +from typing import TYPE_CHECKING + +from tests.project.test_python_versions_1660 import ROOT, prose +from tests.project.test_release_gates import evaluate_expression +from tests.project.test_resource_monitor_1740 import DEFAULT, TESTS, sh, step +from tests.project.test_superseded_runs_1938 import NEWER, OURS, POSIX, REPO, SLEEPER, FakeApi, Files, Handler, \ + Server, main_at +from util import superseded + +if TYPE_CHECKING: + from typing import Any, Dict + +UPLOAD = 'Upload test timings' + + +def upload() -> 'Dict[str, Any]': + return step(UPLOAD) + + +def if_no_files_found(stopped: 'Any') -> 'Any': + """The upload's ``if-no-files-found`` where the tests step's ``stopped`` output is ``stopped``.""" + outputs = {} if stopped is None else {'stopped': stopped} + return evaluate_expression(upload()['with']['if-no-files-found'], {'steps': {'tests': {'outputs': outputs}}}) + + +class TestTheUpload(unittest.TestCase): + + def test_a_stopped_run_expects_no_file_and_any_other_still_warns(self) -> None: + self.assertEqual(if_no_files_found('true'), 'ignore') + for stopped in (None, '', 'false'): + with self.subTest(stopped=stopped): + self.assertEqual(if_no_files_found(stopped), 'warn') + + def test_the_upload_reads_the_tests_steps_junit_files(self) -> None: + """The glob is where ``Run unit tests`` writes junit.xml, and the shards beside it.""" + written = re.findall(r'--junitxml="(\$RUNNER_TEMP/[^"]+)"', step(TESTS)['run']) + self.assertEqual(written, ['$RUNNER_TEMP/junit.xml']) + path = upload()['with']['path'] + self.assertEqual(path, '${{ runner.temp }}/junit*.xml') + self.assertTrue(pathlib.PurePath('junit.xml').match(pathlib.PurePath(path).name)) + self.assertEqual(step(TESTS)['id'], 'tests') + + def test_the_docs_say_it(self) -> None: + page = prose(ROOT / 'docs' / 'source' / 'contributing' / 'workflows.rst') + for phrase in ('`#2073 `__', '``stopped=true``', + '``if-no-files-found: ignore``'): + with self.subTest(phrase=phrase): + self.assertIn(phrase, page) + + +@unittest.skipUnless(POSIX, 'the watcher signals a process group, as on the Ubuntu runners') +class TestTheWatcherSaysItStopped(unittest.TestCase): + + def test_a_stop_writes_stopped_true(self) -> None: + files, out = Files(self), io.StringIO() + code = superseded.main(['watch', '--interval', '0.2', '--grace', '5', '--', *SLEEPER], + env=files.env(), transport=FakeApi(main_at(NEWER)), out=out) + self.assertEqual(code, 0, out.getvalue()) + self.assertEqual(files.read(files.output), 'stopped=true\n') + + def test_a_command_that_ends_on_its_own_writes_nothing(self) -> None: + files, out = Files(self), io.StringIO() + code = superseded.main(['watch', '--interval', '0.2', '--', sys.executable, '-c', 'import sys; sys.exit(3)'], + env=files.env(), transport=FakeApi(main_at(OURS)), out=out) + self.assertEqual(code, 3) + self.assertEqual(files.read(files.output), '') + + def test_an_unwritable_output_does_not_fail_the_stop(self) -> None: + files, out = Files(self), io.StringIO() + env = files.env(GITHUB_OUTPUT=str(files.root / 'missing' / 'output')) + code = superseded.main(['watch', '--interval', '0.2', '--grace', '5', '--', *SLEEPER], + env=env, transport=FakeApi(main_at(NEWER)), out=out) + self.assertEqual(code, 0) + self.assertIn('Could not write the step output: ', out.getvalue()) + + +@unittest.skipUnless(sys.platform.startswith('linux') and all(map(shutil.which, ('bash', 'timeout', 'prlimit'))), + 'the tests step runs coreutils timeout and util-linux prlimit, as on the Ubuntu runners') +class TestTheTestsStepOutput(unittest.TestCase): + """``Run unit tests`` itself, as in test_superseded_runs_1938.py: its ``stopped`` output reaches the upload.""" + + def setUp(self) -> None: + tmp = tempfile.TemporaryDirectory(prefix='pcapkit-2073-') # pylint: disable=consider-using-with + self.addCleanup(tmp.cleanup) + root = pathlib.Path(tmp.name) + self.temp, bindir, self.output = root / 'temp', root / 'bin', root / 'output' + self.temp.mkdir() + bindir.mkdir() + (self.temp / 'resource-monitor.log').write_text('', encoding='utf-8') + self.python = bindir / 'python' + for name, body in (('sudo', 'echo "sudo: a password is required" >&2\nexit 1\n'), + ('python', 'echo collected\nexec sleep 60\n')): + (bindir / name).write_text('#!/bin/sh\n' + body, encoding='utf-8') + (bindir / name).chmod(0o755) + self.server = Server(('127.0.0.1', 0), Handler) + threading.Thread(target=self.server.serve_forever, daemon=True).start() + self.addCleanup(self.server.server_close) + self.addCleanup(self.server.shutdown) + self.env = {key: value for key, value in os.environ.items() if not key.startswith('GITHUB_')} + self.env.update(RUNNER_TEMP=str(self.temp), GITHUB_WORKSPACE=str(ROOT), WORKERS='3', GRAAL_PYTHON_VM_ARGS='', + GITHUB_STEP_SUMMARY=str(root / 'summary.md'), GITHUB_OUTPUT=str(self.output), GH_TOKEN='token', + GITHUB_API_URL=f'http://127.0.0.1:{self.server.server_address[1]}', + GITHUB_EVENT_NAME='push', GITHUB_REF='refs/heads/main', GITHUB_SHA=OURS, + GITHUB_REPOSITORY=REPO, PATH=str(bindir) + os.pathsep + os.environ.get('PATH', ''), + WATCHER='success', WATCHER_PYTHON=sys.executable) + + def script(self) -> 'str': + script = step(TESTS)['run'] + quick = script.replace('watch --interval 60 --grace 30 ', 'watch --interval 0.5 --grace 2 --kill-wait 2 ') + self.assertNotEqual(quick, script) + return quick + + def stopped(self) -> 'Any': + """The ``stopped`` output, ``None`` where the step wrote none.""" + text = self.output.read_text(encoding='utf-8') if self.output.exists() else '' + values = [line.split('=', 1)[1] for line in text.splitlines() if line.startswith('stopped=')] + return values[-1] if values else None + + def test_a_stopped_run_writes_no_junit_and_its_upload_expects_none(self) -> None: + result = sh(self.script(), self.env, DEFAULT) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(list(self.temp.glob('junit*.xml')), []) + self.assertEqual(self.stopped(), 'true') + self.assertEqual(if_no_files_found(self.stopped()), 'ignore') + + def test_a_current_run_still_warns_of_a_missing_file(self) -> None: + self.server.head = OURS # type: ignore[attr-defined] + self.python.write_text('#!/bin/sh\nsleep 1.5\nexit 1\n', encoding='utf-8') + result = sh(self.script(), self.env, DEFAULT) + self.assertEqual(result.returncode, 1, result.stderr) + self.assertIsNone(self.stopped()) + self.assertEqual(if_no_files_found(self.stopped()), 'warn') + + +if __name__ == '__main__': + unittest.main() diff --git a/util/superseded.py b/util/superseded.py index 4973d7fe4d..1fd52c18a5 100644 --- a/util/superseded.py +++ b/util/superseded.py @@ -29,10 +29,11 @@ session or, with ``--scope UNIT``, to every process of that systemd scope (``sudo -n systemctl kill``): ``COMMAND`` then starts a root-owned ``sudo``, and the tests run in the scope as the runner's user, out of reach of a signal to -the session's group (:func:`stop`). Where ``COMMAND`` exits, it writes the notice and the summary -line and exits 0. Otherwise it exits with ``COMMAND``'s own status, ``128 + N`` -for a signal ``N`` as a shell reports it, so a test failure on a current commit -reads as it did without the watcher. SIGINT, SIGTERM and SIGHUP sent to the +the session's group (:func:`stop`). Where ``COMMAND`` exits, it writes the notice, the summary +line and ``stopped=true`` to ``$GITHUB_OUTPUT``, and exits 0: a stopped pytest writes no JUnit +file, so the upload after it reads the output and expects none (#2073). Otherwise it exits +with ``COMMAND``'s own status, ``128 + N`` for a signal ``N`` as a shell reports it, so a test +failure on a current commit reads as it did without the watcher. SIGINT, SIGTERM and SIGHUP sent to the watcher are passed on to ``COMMAND``. Every wait is bounded. Where ``COMMAND`` outlives SIGKILL by ``--kill-wait`` @@ -270,6 +271,17 @@ def gate(context: 'Context', api: 'Api', env: 'Mapping[str, str]', out: 'IO[str] return 0 +def stopped(env: 'Mapping[str, str]', out: 'IO[str]') -> 'None': + """``stopped=true`` in ``$GITHUB_OUTPUT``, so a later step can tell a stopped run from one that ran (#2073).""" + output = env.get('GITHUB_OUTPUT') + if output: + try: + with open(output, 'a', encoding='utf-8') as file: + print('stopped=true', file=file) + except OSError as error: + print(f'Could not write the step output: {error}', file=out, flush=True) + + def exit_status(returncode: 'int') -> 'int': """``returncode`` as a shell reports it: ``128 + N`` for a process ended by signal ``N``.""" return 128 - returncode if returncode < 0 else returncode @@ -379,6 +391,7 @@ def forward(signum: 'int', frame: 'Optional[FrameType]') -> 'None': # pylint: d if stop(child, grace, scope=scope, kill_wait=kill_wait, run=run, kill=kill): elapsed = int(clock() - started) report(verdict, f'The tests were stopped after {elapsed} s, and this step succeeds.', env, out) + stopped(env, out) return 0 where = f'the scope {scope} and ' if scope else '' reason = (f'{verdict.reason}, but SIGTERM and SIGKILL to {where}the session of pid {child.pid} '