diff --git a/.github/workflows/build-and-test-ci.yml b/.github/workflows/build-and-test-ci.yml index 09dd662f66..7a354ff978 100644 --- a/.github/workflows/build-and-test-ci.yml +++ b/.github/workflows/build-and-test-ci.yml @@ -308,7 +308,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 120 container: - image: ghcr.io/edgcpp/ci-runner@sha256:da140ca90c420977f187279d7939fe0c7cb3c4b656e5ab2dc0f6b9ac2a43a255 + image: ghcr.io/edgcpp/ci-runner@sha256:c5c2c5c969f095b6d7d91b8d97f08b359bf1b5b0ed0814996f799e035a659f47 defaults: run: shell: edg-container-exec "bash -eo pipefail {0}" diff --git a/.github/workflows/ci-toolchain.yml b/.github/workflows/ci-toolchain.yml index 1aec80fa48..6099ff85b3 100644 --- a/.github/workflows/ci-toolchain.yml +++ b/.github/workflows/ci-toolchain.yml @@ -66,7 +66,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 120 container: - image: &ci-runner-image ghcr.io/edgcpp/ci-runner@sha256:da140ca90c420977f187279d7939fe0c7cb3c4b656e5ab2dc0f6b9ac2a43a255 + image: &ci-runner-image ghcr.io/edgcpp/ci-runner@sha256:c5c2c5c969f095b6d7d91b8d97f08b359bf1b5b0ed0814996f799e035a659f47 defaults: &container-defaults run: shell: edg-container-exec "bash -eo pipefail {0}" @@ -267,6 +267,17 @@ jobs: edg-prefix-env-loader .tmp/extensive-ci.edgivk + # Reviews (acknowledg-forge, edgy-review) read only the .rt.tar.gz + # files changes.elist names; edgy -A archives every test's. Pruning + # must never keep the results from being uploaded, hence + # continue-on-error. + - name: Prune .rt.tar.gz results changes.elist doesn't name + if: ${{ failure() }} + continue-on-error: true + env: + RUN_TIMESTAMP: ${{ inputs.run_timestamp }} + run: edg-prune-test-run "tests/runs/${RUN_TIMESTAMP}" + - name: Upload partial elist, elog, and .rt.tar.gz results if: ${{ failure() }} uses: actions/upload-artifact@v7 diff --git a/.github/workflows/promote-test-cache.yml b/.github/workflows/promote-test-cache.yml index 1d24f6ab89..821a779a01 100644 --- a/.github/workflows/promote-test-cache.yml +++ b/.github/workflows/promote-test-cache.yml @@ -72,7 +72,7 @@ jobs: # The test legs' image, so that actions/cache picks the same compression # (part of an entry's version) and the test legs can restore the entry. container: - image: ghcr.io/edgcpp/ci-runner@sha256:da140ca90c420977f187279d7939fe0c7cb3c4b656e5ab2dc0f6b9ac2a43a255 + image: ghcr.io/edgcpp/ci-runner@sha256:c5c2c5c969f095b6d7d91b8d97f08b359bf1b5b0ed0814996f799e035a659f47 strategy: fail-fast: false matrix: diff --git a/.github/workflows/strict-build-check.yml b/.github/workflows/strict-build-check.yml index 258e9aa8b6..ff0e11d614 100644 --- a/.github/workflows/strict-build-check.yml +++ b/.github/workflows/strict-build-check.yml @@ -158,7 +158,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 120 container: - image: ghcr.io/edgcpp/ci-runner@sha256:da140ca90c420977f187279d7939fe0c7cb3c4b656e5ab2dc0f6b9ac2a43a255 + image: ghcr.io/edgcpp/ci-runner@sha256:c5c2c5c969f095b6d7d91b8d97f08b359bf1b5b0ed0814996f799e035a659f47 defaults: run: shell: edg-container-exec "bash -eo pipefail {0}" @@ -185,7 +185,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 120 container: - image: ghcr.io/edgcpp/ci-runner@sha256:da140ca90c420977f187279d7939fe0c7cb3c4b656e5ab2dc0f6b9ac2a43a255 + image: ghcr.io/edgcpp/ci-runner@sha256:c5c2c5c969f095b6d7d91b8d97f08b359bf1b5b0ed0814996f799e035a659f47 defaults: run: shell: edg-container-exec "bash -eo pipefail {0}" @@ -212,7 +212,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 120 container: - image: ghcr.io/edgcpp/ci-runner@sha256:da140ca90c420977f187279d7939fe0c7cb3c4b656e5ab2dc0f6b9ac2a43a255 + image: ghcr.io/edgcpp/ci-runner@sha256:c5c2c5c969f095b6d7d91b8d97f08b359bf1b5b0ed0814996f799e035a659f47 defaults: run: shell: edg-container-exec "bash -eo pipefail {0}" @@ -239,7 +239,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 120 container: - image: ghcr.io/edgcpp/ci-runner@sha256:da140ca90c420977f187279d7939fe0c7cb3c4b656e5ab2dc0f6b9ac2a43a255 + image: ghcr.io/edgcpp/ci-runner@sha256:c5c2c5c969f095b6d7d91b8d97f08b359bf1b5b0ed0814996f799e035a659f47 defaults: run: shell: edg-container-exec "bash -eo pipefail {0}" diff --git a/dev_tools/bin/edg-prune-test-run b/dev_tools/bin/edg-prune-test-run new file mode 100755 index 0000000000..451f49fc5a --- /dev/null +++ b/dev_tools/bin/edg-prune-test-run @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 + +# Part of the EDG Compiler Project, under the Apache License v2.0 with LLVM +# Exceptions. +# See https://edgcpp.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +'''Prune a test run down to what a review of it reads, e.g., before CI +uploads the run: + + edg-prune-test-run tests/runs/ + +Reviews of a run read only the .rt.tar files its changes.elist files name. +The replays directory is removed whole. In each configuration, a suite +with no test named there is removed whole (its directory, and its +suite- logs), as is every suite of a configuration without a +changes.elist; of the other suites, the .rt.tar files not named there are +removed. +''' + +import argparse +import sys + +import edgtest +import edgutil + +from pathlib import Path + +def _parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser( + description = ( + "Remove the .rt.tar files of a test run that no configuration's " + 'changes.elist names.' + ) + ) + parser.add_argument( + 'run_dir', + type = Path, + help = 'the run directory (e.g., tests/runs/)' + ) + return parser.parse_args() + +def main() -> int: + args = _parse_args() + if not args.run_dir.is_dir(): + edgutil.eprint(f"error: {args.run_dir} is not a directory") + return 1 + + # Counts: of .rt.tar files kept and removed, and of suites removed. + kept, removed, suites = edgtest.prune_run_archives(args.run_dir) + print( + f"Kept {kept} .rt.tar files named by changes.elist files; removed " + f"{removed} others, and {suites} suites with no changes." + ) + return 0 + +if __name__ == '__main__': + sys.exit(main()) diff --git a/dev_tools/pylibs/edgacknowledg/forge/config.py b/dev_tools/pylibs/edgacknowledg/forge/config.py index 07485371f1..5f10ab8d45 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/config.py +++ b/dev_tools/pylibs/edgacknowledg/forge/config.py @@ -32,8 +32,9 @@ # suite's name, is short. DEFAULT_MAX_PATTERNS = 20_000 # The largest diff (of tests/, in bytes) a pull request may have: it is -# held in memory, and copied and split into lines more than once. -DEFAULT_MAX_DIFF_BYTES = 64 * 1024 * 1024 +# kept with the review's rematerialized run (on disk, never in memory +# whole), and the builder applies all of it. +DEFAULT_MAX_DIFF_BYTES = 512 * 1024 * 1024 # The most entries an artifact may have (its zip's, and its tarballs' # together), and the largest its zip's central directory may be: zipfile # reads all of that directory into memory, before we can count. CI's test diff --git a/dev_tools/pylibs/edgacknowledg/forge/processor.py b/dev_tools/pylibs/edgacknowledg/forge/processor.py index ac17bc9405..7f990d6a1f 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/processor.py +++ b/dev_tools/pylibs/edgacknowledg/forge/processor.py @@ -397,42 +397,54 @@ async def _changes_section(self, if reused is not None: return reused - diff = await self._workspaces.diff( - info.repository, - merge_base, - head_sha, - ['tests/'] - ) - if diff.strip() == '': - return None - - worktree = await self._workspaces.checkout( - info, - record.review_id, - 'base', - merge_base, - changes_patterns( - diff, - max_patterns = self._config.max_sparse_patterns - ) - ) - runs_dir = ensure_real_directory(worktree, PurePosixPath('tests', 'runs')) - run_dir = runs_dir / f"remat-{_sha256(source_key.encode())[:16]}" - remove_tree(run_dir) - # The builder sees the worktree read-only, so it writes elsewhere (on # the same file system, for the rename into place). + run_name = f"remat-{_sha256(source_key.encode())[:16]}" staging_root = ensure_real_directory( self._workspaces.build_staging_dir.parent, PurePosixPath(self._workspaces.build_staging_dir.name) ) staging = Path(tempfile.mkdtemp( - prefix = f"{record.review_id}-{run_dir.name}-", + prefix = f"{record.review_id}-{run_name}-", dir = staging_root )) try: + # Only ever in this file, never held in memory whole: it may be large. diff_file = staging / 'pull-request.diff' - diff_file.write_text(diff, encoding = 'utf-8') + await self._workspaces.diff( + info.repository, + merge_base, + head_sha, + ['tests/'], + diff_file + ) + if diff_file.stat().st_size == 0: + remove_tree(staging) + return None + with open(diff_file, encoding = 'utf-8') as handle: + patterns = changes_patterns( + handle, + max_patterns = self._config.max_sparse_patterns + ) + + worktree = await self._workspaces.checkout( + info, + record.review_id, + 'base', + merge_base, + patterns + ) + # Prepare where the run goes once built: tests/runs/ in the + # worktree (through real directories only, not links the pull request + # committed), cleared of any earlier attempt so that the staging + # directory can be renamed into place. + runs_dir = ensure_real_directory( + worktree, + PurePosixPath('tests', 'runs') + ) + run_dir = runs_dir / run_name + remove_tree(run_dir) + timestamp = self._clock().strftime(edgutil.RUN_TIMESTAMP_FORMAT) result = await self._sandbox.run( [ @@ -477,6 +489,7 @@ async def _changes_section(self, ) remove_tree(staging) return None + fingerprint = await asyncio.to_thread(_file_sha256, diff_file) staging.rename(run_dir) except BaseException: remove_tree(staging) @@ -487,7 +500,7 @@ async def _changes_section(self, name = CHANGES_NAME, slug = section_slug(CHANGES_NAME), source_key = source_key, - content_fingerprint = _sha256(diff.encode()), + content_fingerprint = fingerprint, root = run_dir / timestamp, home = worktree / 'tests', details = {'run_dir': str(run_dir)}, diff --git a/dev_tools/pylibs/edgacknowledg/forge/workspace.py b/dev_tools/pylibs/edgacknowledg/forge/workspace.py index 6079f52b22..f7f56d63b4 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/workspace.py +++ b/dev_tools/pylibs/edgacknowledg/forge/workspace.py @@ -26,6 +26,7 @@ import asyncio import base64 +import codecs import logging import os import re @@ -39,7 +40,7 @@ from dataclasses import dataclass from pathlib import Path, PurePosixPath -from typing import Iterable, Optional, Sequence +from typing import Callable, Iterable, Optional, Sequence import edgtest import edgutil @@ -81,6 +82,10 @@ class UnsafeArchiveError(WorkspaceError): '''Raised for an artifact that is too large or has unsafe entries.''' +class _OutputTooLarge(Exception): + '''Raised when a process's output is over its limit (it is stopped).''' + + @dataclass(frozen = True) class GitResult: returncode: int @@ -140,16 +145,18 @@ def changed_paths(diff: str) -> list[str]: return paths -def changes_patterns(diff: str, +def changes_patterns(lines_iter: Iterable[str], *, max_patterns: int = DEFAULT_MAX_PATTERNS) -> list[str]: - '''Sparse-checkout patterns for rematerializing ``diff`` (of tests/) on - the base tree: the test home and every affected test; at most + '''Sparse-checkout patterns for rematerializing the diff (of tests/) of + ``lines_iter`` (with or without their line ends, e.g. a file's) on the + base tree: the test home and every affected test; at most ``max_patterns``. ''' patterns = dict.fromkeys(_TEST_HOME_PATTERNS) current_suite: Optional[str] = None - for line in diff.splitlines(): + for line in lines_iter: + line = line.rstrip('\n') match = _DIFF_HEADER.match(line) if match is not None: path = PurePosixPath(match.group(1)) @@ -478,9 +485,13 @@ async def git(self, cwd: Optional[Path] = None, stdin: Optional[str] = None, check: bool = True, - max_output: Optional[int] = None) -> GitResult: - '''Run git; with ``max_output``, its output may be at most that many - bytes (it is stopped, and ``WorkspaceError`` raised, beyond). + max_output: Optional[int] = None, + stdout_sink: Optional[Callable[[bytes], None]] = None + ) -> GitResult: + '''Run git; with ``stdout_sink``, its output goes there as it comes, + rather than into the result, and may be at most ``max_output`` bytes + (it is stopped, and ``_OutputTooLarge`` raised, beyond: the caller + knows which limit that is). ''' command = ['git'] if git_dir is not None: @@ -497,18 +508,15 @@ async def git(self, cwd = cwd, env = self._git_env() ) - if max_output is None: + if stdout_sink is None: + assert max_output is None stdout, stderr = await process.communicate( stdin.encode() if stdin is not None else None ) else: - assert stdin is None - bounded = await _communicate_bounded(process, max_output) - if bounded is None: - raise WorkspaceError( - f"git {args[0]}'s output is over {max_output} bytes" - ) - stdout, stderr = bounded + assert stdin is None and max_output is not None + stderr = await _communicate_bounded(process, max_output, stdout_sink) + stdout = b'' assert process.returncode is not None result = GitResult( process.returncode, @@ -609,15 +617,42 @@ async def diff(self, repository: str, base: str, head: str, - paths: Sequence[str]) -> str: + paths: Sequence[str], + destination: Path) -> None: + '''Write the diff of ``paths`` from ``base`` to ``head`` to (the new + file) ``destination``, as it comes (it may be large): UTF-8, with what + isn't replaced (U+FFFD). + ''' + decoder = codecs.getincrementaldecoder('utf-8')(errors = 'replace') async with self.lock(repository): - result = await self.git( - 'diff', '--no-renames', '--no-color', '--no-ext-diff', '--no-textconv', - base, head, '--', *paths, - git_dir = self.clone_dir(repository), - max_output = self._max_diff_bytes - ) - return result.stdout + with open(destination, 'xb') as handle: + + # Decoded and encoded again so that what isn't UTF-8 is replaced: + # the builder's edg-rematerialize-test-run (and changes_patterns) + # read it as UTF-8 text, and the Changes section's fingerprint + # (the file's) stays what it was when the diff was decoded whole. + # Incrementally, so a character split between chunks survives. + + def write(chunk: bytes) -> None: + handle.write(decoder.decode(chunk).encode()) + + try: + await self.git( + 'diff', '--no-renames', '--no-color', '--no-ext-diff', + '--no-textconv', base, head, '--', *paths, + git_dir = self.clone_dir(repository), + max_output = self._max_diff_bytes, + stdout_sink = write + ) + except _OutputTooLarge: + raise WorkspaceError( + f"the diff is over {self._max_diff_bytes} bytes " + f"({ENV_PREFIX}MAX_DIFF_BYTES)" + ) from None + + # Finish decoding: bytes the decoder still holds (a character the + # output ended in the middle of) are written too, replaced. + handle.write(decoder.decode(b'', final = True).encode()) async def checkout(self, info: PullRequestInfo, @@ -702,31 +737,31 @@ async def collect_garbage(self, repository: str, open_count: int) -> None: async def _communicate_bounded(process: asyncio.subprocess.Process, - limit: int) -> Optional[tuple[bytes, bytes]]: - '''``process``'s output (stdout, stderr), once it exits; ``None``, having - stopped it, should its stdout be over ``limit`` bytes. Should this be - cancelled (or fail), it is stopped too, rather than left writing to a - pipe no one reads. + limit: int, + sink: Callable[[bytes], None]) -> bytes: + '''Pass ``process``'s stdout to ``sink`` as it comes, in chunks (none held + here); return its stderr once it exits. Should its stdout be over + ``limit`` bytes, it is stopped, and ``_OutputTooLarge`` raised. Should + this be cancelled (or fail), it is stopped too, rather than left writing + to a pipe no one reads. ''' assert process.stdout is not None and process.stderr is not None - errors = asyncio.create_task(process.stderr.read()) - chunks: list[bytes] = [] + reading_stderr = asyncio.create_task(process.stderr.read()) size = 0 try: while chunk := await process.stdout.read(64 * 1024): size += len(chunk) if size > limit: - await edgutil.async_terminate_within(process, _TERMINATE_ALLOWANCE) - return None - chunks.append(chunk) - output = b''.join(chunks), await errors + raise _OutputTooLarge() + sink(chunk) + stderr = await reading_stderr await process.wait() - return output + return stderr except BaseException: await edgutil.async_terminate_within(process, _TERMINATE_ALLOWANCE) raise finally: - errors.cancel() + reading_stderr.cancel() def basic_auth_header(user: str, password: str) -> str: diff --git a/dev_tools/pylibs/edgtest/__init__.py b/dev_tools/pylibs/edgtest/__init__.py index 0b8ef00085..07ce2a675c 100644 --- a/dev_tools/pylibs/edgtest/__init__.py +++ b/dev_tools/pylibs/edgtest/__init__.py @@ -306,6 +306,86 @@ def find_changes_files(run_dir: Path) -> Dict[str, Path]: changes_files[dir_entry] = changes_file return changes_files +def _remove_run_tree(path: Path) -> int: + '''Remove the directory path (of a test run) whole; return how many + .rt.tar files it held. + ''' + count = 0 + for _, _, file_names in os.walk(path): + count += sum( + 1 for file_name in file_names + if file_name.endswith(runtest.RUN_TEST_OUTPUT_SUFFIX) + ) + shutil.rmtree(path) + return count + +def prune_run_archives(run_dir: Path) -> Tuple[int, int, int]: + '''Prune the run in run_dir down to what a review of it reads (the .rt.tar + files changes.elist names; see _scan_changes_file), e.g., to keep CI's + uploads small. The replays directory is removed whole. In each + configuration, a suite with no test changes.elist names (every suite, + without a changes.elist) is removed whole: its directory, and its + suite- logs. Of the other suites, the .rt.tar files changes.elist + doesn't name are removed. Return how many .rt.tar files were kept, how + many removed, and how many suites were removed. + ''' + kept = 0 + removed = 0 + suites = 0 + for dir_entry in os.listdir(run_dir): + config_dir = run_dir / dir_entry + if not config_dir.is_dir() or config_dir.is_symlink(): + continue + if dir_entry == 'replays': + removed += _remove_run_tree(config_dir) + continue + if dir_entry in ('recording-updates', 'patched-tests'): + continue + + # The .rt.tar files changes.elist names, by suite. + named: Dict[str, Set[Path]] = {} + changes_file = config_dir / 'changes.elist' + if changes_file.is_file(): + with open(changes_file, 'r') as file_handle: + test_ids = [ + test_id for test_id in (line.strip() for line in file_handle) + if test_id != '' + ] + for test_id in test_ids: + test_path = PurePosixPath(test_id) + named.setdefault(test_path.parts[0], set()).add( + config_dir / test_path.with_suffix(runtest.RUN_TEST_OUTPUT_SUFFIX) + ) + + # Each suite edgy ran (and logged) in the configuration. + for log_path in config_dir.glob('suite-*.elog'): + suite = log_path.stem[len('suite-'):] + suite_dir = config_dir / suite + walkable = suite_dir.is_dir() and not suite_dir.is_symlink() + if suite not in named: + if walkable: + removed += _remove_run_tree(suite_dir) + log_path.unlink() + try: + log_path.with_suffix('.log').unlink() + except FileNotFoundError: + pass + suites += 1 + continue + if not walkable: + continue + for root, _, file_names in os.walk(suite_dir): + for file_name in file_names: + if not file_name.endswith(runtest.RUN_TEST_OUTPUT_SUFFIX): + continue + file_path = Path(root, file_name) + if file_path in named[suite]: + kept += 1 + else: + file_path.unlink() + removed += 1 + return kept, removed, suites + def describe_rt_path(rt_file_path: Path, run_dir: Path) -> Tuple[str, str]: '''Return (relative_rt_path, config_name) for an .rt.tar under the run. diff --git a/dev_tools/services/acknowledg-forge/README.md b/dev_tools/services/acknowledg-forge/README.md index 1568da7961..87edb3054a 100644 --- a/dev_tools/services/acknowledg-forge/README.md +++ b/dev_tools/services/acknowledg-forge/README.md @@ -114,7 +114,7 @@ from the environment variable shown as a fallback. | `ACKNOWLEDG_FORGE_MAX_EXTRACTED_BYTES` | 2 GiB | Largest extracted artifact (including inner tarballs) | | `ACKNOWLEDG_FORGE_MAX_ARTIFACT_ENTRIES` | `500000` | Most entries in an artifact (including inner tarballs'). CI's test results reach about 170,000 | | `ACKNOWLEDG_FORGE_MAX_ARTIFACT_DIRECTORY_BYTES` | 64 MiB | Largest artifact zip central directory, which is read into memory whole (about 106 bytes per entry in CI's test results) | -| `ACKNOWLEDG_FORGE_MAX_DIFF_BYTES` | 64 MiB | Largest diff of a pull request's `tests/`, which is held in memory and copied more than once: raising it may need a higher `mem_limit` for the service ([`compose.yaml`](compose.yaml)) | +| `ACKNOWLEDG_FORGE_MAX_DIFF_BYTES` | 512 MiB | Largest diff of a pull request's `tests/`. It is streamed to disk, not held in memory, and kept with the review's rematerialized run; the builder applies all of it within `ACKNOWLEDG_FORGE_REMATERIALIZE_TIMEOUT` | | `ACKNOWLEDG_FORGE_MAX_SPARSE_PATTERNS` | `20000` | Most sparse-checkout patterns in a review's worktree (up to two per test). Checking out takes time proportional to patterns times paths | | `ACKNOWLEDG_FORGE_MAX_CONCURRENCY` | `2` | Pull requests processed at once | | `ACKNOWLEDG_FORGE_REMATERIALIZE_TIMEOUT` | `900` | Seconds `edg-rematerialize-test-run` may run | diff --git a/dev_tools/services/acknowledg-forge/SANDBOXING.md b/dev_tools/services/acknowledg-forge/SANDBOXING.md index c89d1401a9..2cc095e27d 100644 --- a/dev_tools/services/acknowledg-forge/SANDBOXING.md +++ b/dev_tools/services/acknowledg-forge/SANDBOXING.md @@ -527,7 +527,7 @@ them. Several layers keep the service from reading through one: | --- | --- | | Artifact download | `ACKNOWLEDG_FORGE_MAX_ARTIFACT_BYTES` (256 MiB), also counting the decompressed size of inner tarballs | | Artifact entries | `ACKNOWLEDG_FORGE_MAX_ARTIFACT_ENTRIES` (500,000), counting inner tarballs' too; and its zip's central directory, `ACKNOWLEDG_FORGE_MAX_ARTIFACT_DIRECTORY_BYTES` (64 MiB), checked before zipfile reads it | -| A pull request's diff of `tests/` | `ACKNOWLEDG_FORGE_MAX_DIFF_BYTES` (64 MiB) | +| A pull request's diff of `tests/` | `ACKNOWLEDG_FORGE_MAX_DIFF_BYTES` (512 MiB) | | A review worktree's sparse-checkout patterns | `ACKNOWLEDG_FORGE_MAX_SPARSE_PATTERNS` (20,000) | | A file read for a reviewer | 64 MiB; a larger one fails the request | | A source shown to a reviewer | only as much as can be displayed is read | diff --git a/dev_tools/services/acknowledg-forge/tests/test_safety.py b/dev_tools/services/acknowledg-forge/tests/test_safety.py index 822538d76a..5d27f9f93c 100644 --- a/dev_tools/services/acknowledg-forge/tests/test_safety.py +++ b/dev_tools/services/acknowledg-forge/tests/test_safety.py @@ -23,6 +23,8 @@ from edgacknowledg.forge.workspace import ( UnsafeArchiveError, WorkspaceError, + Workspaces, + _OutputTooLarge, _central_directory_size, _communicate_bounded, changes_patterns, @@ -32,6 +34,9 @@ patterns_for_test, ) +from conftest import Origin, pull_request_info +from fakes import REPOSITORY, FakeForge + def _zip(path: Path, entries: dict[str, bytes]) -> Path: with zipfile.ZipFile(path, 'w') as archive: for name, data in entries.items(): @@ -226,7 +231,7 @@ def test_changes_patterns_cover_recordings_and_expectations() -> None: '-dir/b.sft.cpp: PASS', '+dir/b.sft.cpp: FAIL', ]) - patterns = changes_patterns(diff) + patterns = changes_patterns(diff.splitlines()) assert '/tests/tests/s/.a.rto/' in patterns assert '/tests/tests/s/a.*' in patterns assert '/tests/tests/other/dir/b.sft.cpp' in patterns @@ -254,10 +259,10 @@ def expectations(suite: str) -> str: '+a.sft.cpp: PASS', ]) - patterns = changes_patterns(expectations('cwg')) + patterns = changes_patterns(expectations('cwg').splitlines()) assert '/tests/tests/cwg/a.sft.cpp' in patterns for suite in ['s' * 65, 'a.b', 'a b']: - assert changes_patterns(expectations(suite)) == [ + assert changes_patterns(expectations(suite).splitlines()) == [ '/tests/.edgy/', '/tests/expectations/', ] @@ -276,34 +281,80 @@ def test_patterns_are_capped() -> None: *(f"+{n}.sft.cpp: FAIL" for n in range(cap)), ]) with pytest.raises(WorkspaceError, match = 'sparse-checkout patterns'): - changes_patterns(diff, max_patterns = cap) + changes_patterns(diff.splitlines(), max_patterns = cap) # Within a higher cap. - changes_patterns(diff, max_patterns = 3 * cap) + changes_patterns(diff.splitlines(), max_patterns = 3 * cap) async def test_git_output_can_be_bounded() -> None: - async def run(*argv: str, limit: int) -> object: - process = await asyncio.create_subprocess_exec( - *argv, - stdout = asyncio.subprocess.PIPE, - stderr = asyncio.subprocess.PIPE - ) - result = await _communicate_bounded(process, limit) - assert process.returncode is not None - return result - - assert await run('sh', '-c', 'printf out; printf err >&2', - limit = 3) == (b'out', b'err') + # Stdout goes to the sink as it comes; stderr is returned. + sunk: list[bytes] = [] + process = await asyncio.create_subprocess_exec( + 'sh', '-c', 'printf out; printf err >&2', + stdout = asyncio.subprocess.PIPE, + stderr = asyncio.subprocess.PIPE + ) + assert await _communicate_bounded(process, 3, sunk.append) == b'err' + assert process.returncode is not None + assert b''.join(sunk) == b'out' # One that would never stop is stopped (letting it clean up). process = await asyncio.create_subprocess_exec( 'yes', stdout = asyncio.subprocess.PIPE, stderr = asyncio.subprocess.PIPE ) - assert await _communicate_bounded(process, 1024 * 1024) is None + with pytest.raises(_OutputTooLarge): + await _communicate_bounded(process, 1024 * 1024, sunk.append) assert process.returncode == -signal.SIGTERM +async def test_diffs_are_written_as_they_come(tmp_path: Path, + origin: Origin, + forge: FakeForge) -> None: + # Longer than a chunk git's output comes in, with characters of several + # bytes across them. + lines = [f"{n}.sft.cpp: {'é€😀' * 10}" for n in range(2000)] + head = origin.push_pull_request(1, { + 'tests/expectations/x/s.log': ''.join(f"{line}\n" for line in lines), + }) + workspaces = Workspaces(tmp_path / 'home', forge) + info = pull_request_info(1, head, origin.main_sha) + head, _ = await workspaces.fetch_pull_request(info) + diff_file = tmp_path / 'pull-request.diff' + await workspaces.diff( + REPOSITORY, + origin.main_sha, + head, + ['tests/'], + diff_file + ) + diff = diff_file.read_text(encoding = 'utf-8') + assert len(diff_file.read_bytes()) > 64 * 1024 + assert '\ufffd' not in diff + added = [ + line[1:] for line in diff.splitlines() + if line.startswith('+') and not line.startswith('+++') + ] + assert added == lines + # Its patterns, read from the file as from the text. + with open(diff_file, encoding = 'utf-8') as handle: + assert changes_patterns(handle) == changes_patterns(diff.splitlines()) + assert '/tests/tests/s/1999.sft.cpp' in changes_patterns(diff.splitlines()) + + # Too long: refused. + workspaces = Workspaces(tmp_path / 'home', forge, max_diff_bytes = 1024) + with pytest.raises( + WorkspaceError, + match = r'over 1024 bytes \(ACKNOWLEDG_FORGE_MAX_DIFF_BYTES\)'): + await workspaces.diff( + REPOSITORY, + origin.main_sha, + head, + ['tests/'], + tmp_path / 'too-long.diff' + ) + + async def test_cancelled_git_output_reads_stop_git() -> None: # As when the service stops while git still has output to write: left # writing to a pipe no one reads, it would never exit. @@ -312,7 +363,9 @@ async def test_cancelled_git_output_reads_stop_git() -> None: stdout = asyncio.subprocess.PIPE, stderr = asyncio.subprocess.PIPE ) - reading = asyncio.create_task(_communicate_bounded(process, 1 << 40)) + reading = asyncio.create_task( + _communicate_bounded(process, 1 << 40, lambda chunk: None) + ) await asyncio.sleep(0.2) reading.cancel() await asyncio.wait_for(asyncio.gather(reading, return_exceptions = True), 10) diff --git a/dev_tools/services/acknowledg-forge/tests/test_state.py b/dev_tools/services/acknowledg-forge/tests/test_state.py index 2b45050ae7..b1149425f1 100644 --- a/dev_tools/services/acknowledg-forge/tests/test_state.py +++ b/dev_tools/services/acknowledg-forge/tests/test_state.py @@ -427,7 +427,7 @@ def test_webhooks_need_the_service_bind(tmp_path: Path) -> None: @pytest.mark.parametrize('setting, problem', [ - ('MAX_DIFF_BYTES', "git diff's output is over 1 bytes"), + ('MAX_DIFF_BYTES', 'MAX_DIFF_BYTES'), ('MAX_SPARSE_PATTERNS', 'MAX_SPARSE_PATTERNS'), ('MAX_ARTIFACT_ENTRIES', 'MAX_ARTIFACT_ENTRIES'), ('MAX_ARTIFACT_DIRECTORY_BYTES', 'MAX_ARTIFACT_DIRECTORY_BYTES'), diff --git a/dev_tools/tests/test_edgtest_prune.py b/dev_tools/tests/test_edgtest_prune.py new file mode 100644 index 0000000000..06ec2a4500 --- /dev/null +++ b/dev_tools/tests/test_edgtest_prune.py @@ -0,0 +1,122 @@ +# Part of the EDG Compiler Project, under the Apache License v2.0 with LLVM +# Exceptions. +# See https://edgcpp.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +'''Tests for pruning a test run's .rt.tar files (edgtest.prune_run_archives) +and edg-prune-test-run. + +Run with: python3 -m unittest discover -s dev_tools/tests +(with dev_tools/pylibs on PYTHONPATH). +''' + +import os +import subprocess +import sys +import tempfile +import unittest + +from pathlib import Path +from typing import List + +import edgtest + +from edgtest import runtest + +DEV_TOOLS_DIR = Path(__file__).resolve().parents[1] +PRUNE_TOOL_PATH = DEV_TOOLS_DIR / 'bin' / 'edg-prune-test-run' + +SUFFIX = runtest.RUN_TEST_OUTPUT_SUFFIX + +def _touch(path: Path) -> Path: + path.parent.mkdir(parents = True, exist_ok = True) + path.write_bytes(b'') + return path + +def _files(root: Path) -> List[str]: + return sorted( + Path(directory, name).relative_to(root).as_posix() + for directory, _, names in os.walk(root) + for name in names + ) + +class PruneRunArchivesTests(unittest.TestCase): + + def setUp(self) -> None: + temp_dir = tempfile.TemporaryDirectory() + self.addCleanup(temp_dir.cleanup) + self.run_dir = Path(temp_dir.name) / 'run' + # One configuration whose changes.elist names two tests of one suite + # (and one without results), with every test's results... + config = self.run_dir / 'edg_x86_64' + _touch(config / 'changes.elist').write_text( + 'suite/a.sft.cpp\nsuite/dir/b.sft.c\nsuite/none.sft.cpp\n\n' + ) + for name in ('changes.elog', 'executed.elist', 'suite-suite.elog', + 'suite-suite.log', 'suite-other.elog', 'suite-other.log'): + _touch(config / name) + for name in ('suite/a.sft', 'suite/dir/b.sft', 'suite/c.sft', + 'other/a.sft'): + _touch(config / f"{name}{SUFFIX}") + # ...one without a changes.elist... + config = self.run_dir / 'edg_x86_32' + for name in ('executed.elist', 'suite-suite.elog', 'suite-suite.log', + f"suite/a.sft{SUFFIX}"): + _touch(config / name) + # ...and the run's replays (removed whole). + _touch(self.run_dir / 'replays' / f"suite/c.sft{SUFFIX}") + + def test_keeps_only_what_changes_elist_names(self) -> None: + changes_file = self.run_dir / 'edg_x86_64' / 'changes.elist' + read = list(edgtest._scan_changes_file(changes_file)) + + self.assertEqual(edgtest.prune_run_archives(self.run_dir), (2, 4, 2)) + # Suites with no changes go whole, and unnamed results with them. + self.assertEqual(_files(self.run_dir), [ + 'edg_x86_32/executed.elist', + 'edg_x86_64/changes.elist', + 'edg_x86_64/changes.elog', + 'edg_x86_64/executed.elist', + 'edg_x86_64/suite-suite.elog', + 'edg_x86_64/suite-suite.log', + 'edg_x86_64/suite/a.sft' + SUFFIX, + 'edg_x86_64/suite/dir/b.sft' + SUFFIX, + ]) + # A review reads what it read before. + self.assertEqual(list(edgtest._scan_changes_file(changes_file)), read) + + # Pruning again changes nothing. + self.assertEqual(edgtest.prune_run_archives(self.run_dir), (2, 0, 0)) + + def test_keeps_nothing_without_changes_elist(self) -> None: + (self.run_dir / 'edg_x86_64' / 'changes.elist').unlink() + self.assertEqual(edgtest.prune_run_archives(self.run_dir), (0, 6, 3)) + self.assertEqual(_files(self.run_dir), [ + 'edg_x86_32/executed.elist', + 'edg_x86_64/changes.elog', + 'edg_x86_64/executed.elist', + ]) + + def test_command(self) -> None: + env = dict(os.environ) + env['PYTHONPATH'] = str(DEV_TOOLS_DIR / 'pylibs') + result = subprocess.run( + [sys.executable, str(PRUNE_TOOL_PATH), str(self.run_dir)], + capture_output = True, + text = True, + env = env + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn('Kept 2', result.stdout) + self.assertIn('removed 4 others, and 2 suites', result.stdout) + + missing = subprocess.run( + [sys.executable, str(PRUNE_TOOL_PATH), str(self.run_dir / 'missing')], + capture_output = True, + text = True, + env = env + ) + self.assertEqual(missing.returncode, 1) + +if __name__ == '__main__': + unittest.main()