From 66607c414e0d3eddffe71b90e588290deb599d14 Mon Sep 17 00:00:00 2001 From: Wyatt Childers Date: Tue, 3 Nov 2026 16:13:21 +0000 Subject: [PATCH] Make the forge's input limits settings, raising the artifact ones [] The artifact entry, central directory, diff, and sparse-checkout pattern caps are now settings (ACKNOWLEDG_FORGE_MAX_ARTIFACT_ENTRIES, ACKNOWLEDG_FORGE_MAX_ARTIFACT_DIRECTORY_BYTES, ACKNOWLEDG_FORGE_MAX_DIFF_BYTES, and ACKNOWLEDG_FORGE_MAX_SPARSE_PATTERNS), and their errors name the setting to raise. The artifact defaults rise to 500,000 entries and 64 MiB, and the diff's to 64 MiB. --- .../pylibs/edgacknowledg/forge/config.py | 38 ++++++ .../pylibs/edgacknowledg/forge/processor.py | 14 ++- .../pylibs/edgacknowledg/forge/service.py | 6 +- .../pylibs/edgacknowledg/forge/workspace.py | 116 ++++++++++-------- dev_tools/services/acknowledg-forge/README.md | 4 + .../services/acknowledg-forge/SANDBOXING.md | 3 + .../acknowledg-forge/tests/conftest.py | 6 +- .../acknowledg-forge/tests/test_safety.py | 56 ++++++--- .../acknowledg-forge/tests/test_state.py | 32 +++++ 9 files changed, 204 insertions(+), 71 deletions(-) diff --git a/dev_tools/pylibs/edgacknowledg/forge/config.py b/dev_tools/pylibs/edgacknowledg/forge/config.py index af6e034249..07485371f1 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/config.py +++ b/dev_tools/pylibs/edgacknowledg/forge/config.py @@ -20,6 +20,27 @@ ENV_PREFIX = 'ACKNOWLEDG_FORGE_' +# The defaults of the limits on what a pull request and its CI artifacts +# may make the service do (``workspace``), which the settings may raise for +# large ones, given the memory (and time) they take. +# +# The most sparse-checkout patterns a checkout may have: matching costs +# patterns x paths, and they are built from pull request content. Their +# total size must also stay within a small multiple of the diff's (see +# DEFAULT_MAX_DIFF_BYTES): a test gets at most two patterns, each about as +# long as its path, and the only text that is part of many patterns, a +# 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 +# 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 +# results reach about 170,000 entries, of about 106 bytes each there. +DEFAULT_MAX_ARTIFACT_ENTRIES = 500_000 +DEFAULT_MAX_ARTIFACT_DIRECTORY_BYTES = 64 * 1024 * 1024 + class ConfigError(Exception): '''Raised for a missing or malformed setting.''' @@ -102,6 +123,10 @@ class ForgeServiceConfig: bench_artifact: str max_artifact_bytes: int max_extracted_bytes: int + max_artifact_entries: int + max_artifact_directory_bytes: int + max_diff_bytes: int + max_sparse_patterns: int max_concurrency: int rematerialize_timeout: float unsandboxed: bool @@ -240,6 +265,19 @@ def get_list(name: str) -> tuple[str, ...]: max_extracted_bytes = int( get_number('MAX_EXTRACTED_BYTES', 2 * 1024 * 1024 * 1024) ), + max_artifact_entries = int( + get_number('MAX_ARTIFACT_ENTRIES', DEFAULT_MAX_ARTIFACT_ENTRIES) + ), + max_artifact_directory_bytes = int(get_number( + 'MAX_ARTIFACT_DIRECTORY_BYTES', + DEFAULT_MAX_ARTIFACT_DIRECTORY_BYTES + )), + max_diff_bytes = int( + get_number('MAX_DIFF_BYTES', DEFAULT_MAX_DIFF_BYTES) + ), + max_sparse_patterns = int( + get_number('MAX_SPARSE_PATTERNS', DEFAULT_MAX_PATTERNS) + ), max_concurrency = max_concurrency, rematerialize_timeout = get_number('REMATERIALIZE_TIMEOUT', 15 * 60), unsandboxed = unsandboxed, diff --git a/dev_tools/pylibs/edgacknowledg/forge/processor.py b/dev_tools/pylibs/edgacknowledg/forge/processor.py index f4b7727be2..ac17bc9405 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/processor.py +++ b/dev_tools/pylibs/edgacknowledg/forge/processor.py @@ -411,7 +411,10 @@ async def _changes_section(self, record.review_id, 'base', merge_base, - changes_patterns(diff) + 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]}" @@ -509,7 +512,9 @@ async def _stage_artifact(self, extract_artifact, zip_path, staging, - max_bytes = self._config.max_extracted_bytes + max_bytes = self._config.max_extracted_bytes, + max_entries = self._config.max_artifact_entries, + max_directory_bytes = self._config.max_artifact_directory_bytes ) finally: zip_path.unlink(missing_ok = True) @@ -649,7 +654,10 @@ async def _build_ci_sections(self, record.review_id, 'head', tested_sha, - failures_patterns(failed_tests) + failures_patterns( + failed_tests, + max_patterns = self._config.max_sparse_patterns + ) ) sections: list[SectionRecord] = [] diff --git a/dev_tools/pylibs/edgacknowledg/forge/service.py b/dev_tools/pylibs/edgacknowledg/forge/service.py index dc2b288afa..35516802eb 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/service.py +++ b/dev_tools/pylibs/edgacknowledg/forge/service.py @@ -104,7 +104,11 @@ async def _run_components(config: ForgeServiceConfig, try: wake = asyncio.Event() ingestor = Ingestor(store, forge, config, wake = wake) - workspaces = Workspaces(config.home, forge) + workspaces = Workspaces( + config.home, + forge, + max_diff_bytes = config.max_diff_bytes + ) # Serving reviewers happens in the publisher container. publisher = PublisherClient( url = publisher_service_url, diff --git a/dev_tools/pylibs/edgacknowledg/forge/workspace.py b/dev_tools/pylibs/edgacknowledg/forge/workspace.py index 3b8f30eaff..6079f52b22 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/workspace.py +++ b/dev_tools/pylibs/edgacknowledg/forge/workspace.py @@ -45,6 +45,13 @@ import edgutil from edgacknowledg.forge import Forge, PullRequestInfo +from edgacknowledg.forge.config import ( + DEFAULT_MAX_ARTIFACT_DIRECTORY_BYTES, + DEFAULT_MAX_ARTIFACT_ENTRIES, + DEFAULT_MAX_DIFF_BYTES, + DEFAULT_MAX_PATTERNS, + ENV_PREFIX, +) from edgacknowledg.forge.runner.roles import BUILD_STAGING_DIR, REVIEWS_DIR from edgtest.runtest import get_bare_name @@ -61,21 +68,6 @@ BENCH_BASELINE_PATTERNS = ('/benchmarks/baselines/',) -# The most sparse-checkout patterns a checkout may have: matching costs -# patterns x paths, and they are built from pull request content. Their -# total size must also stay within a small multiple of the diff's (see -# MAX_DIFF_BYTES): a test gets at most two patterns, each about as long as -# its path, and the only text that is part of many patterns, a suite's -# name, is short. -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. -MAX_DIFF_BYTES = 16 * 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. -MAX_ARTIFACT_ENTRIES = 100_000 -MAX_CENTRAL_DIRECTORY_BYTES = 16 * 1024 * 1024 # Seconds a git process being stopped has to clean up (e.g. remove its lock # files) before it is killed; well within docker stop's default 10. _TERMINATE_ALLOWANCE = 5.0 @@ -106,7 +98,7 @@ def patterns_for_test(test_path: PurePosixPath) -> list[str]: as 'tests/tests/suite/dir/name.sft.cpp'): the test file and its recordings (where the test runner keeps them, '..rto'), or the whole directory of a multi-file test. At most two, each about as long as the - path, however it is named (see MAX_PATTERNS). + path, however it is named (see config.DEFAULT_MAX_PATTERNS). ''' directory = _escape_pattern(str(test_path.parent)) if '.mft.' in test_path.name: @@ -148,9 +140,12 @@ def changed_paths(diff: str) -> list[str]: return paths -def changes_patterns(diff: str) -> list[str]: +def changes_patterns(diff: 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. + 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 @@ -160,12 +155,16 @@ def changes_patterns(diff: str) -> list[str]: path = PurePosixPath(match.group(1)) current_suite = None if path.parts[:2] == ('tests', 'tests'): - _add_patterns(patterns, _patterns_for_changed_test_file(path)) + _add_patterns( + patterns, + _patterns_for_changed_test_file(path), + max_patterns + ) elif (path.parts[:2] == ('tests', 'expectations') and path.suffix == '.log'): # Expectation lines name tests relative to the suite, so its name is - # part of the patterns of each (see MAX_PATTERNS). A file whose name - # no suite may have (e.g. too long) names no tests. + # part of the patterns of each (see config.DEFAULT_MAX_PATTERNS). A + # file whose name no suite may have (e.g. too long) names no tests. if edgtest.is_valid_suite_name(path.stem): current_suite = path.stem continue @@ -174,33 +173,43 @@ def changes_patterns(diff: str) -> list[str]: if line.startswith(('-', '+')): test_name = line[1:].split(':')[0].strip() if test_name: - _add_patterns(patterns, patterns_for_test( - PurePosixPath('tests', 'tests', current_suite, test_name) - )) + _add_patterns( + patterns, + patterns_for_test( + PurePosixPath('tests', 'tests', current_suite, test_name) + ), + max_patterns + ) return list(patterns) -def failures_patterns(test_ids: Iterable[str]) -> list[str]: +def failures_patterns(test_ids: Iterable[str], + *, + max_patterns: int = DEFAULT_MAX_PATTERNS) -> list[str]: '''Sparse-checkout patterns for serving CI results naming ``test_ids`` - ('/', as in .elist files), plus bench baselines. + ('/', as in .elist files), plus bench baselines; at most + ``max_patterns``. ''' patterns = dict.fromkeys([*_TEST_HOME_PATTERNS, *BENCH_BASELINE_PATTERNS]) for test_id in test_ids: test_path = PurePosixPath('tests', 'tests', test_id) if '..' in test_path.parts or test_path.is_absolute(): continue - _add_patterns(patterns, patterns_for_test(test_path)) + _add_patterns(patterns, patterns_for_test(test_path), max_patterns) return list(patterns) -def _add_patterns(patterns: dict[str, None], more: list[str]) -> None: +def _add_patterns(patterns: dict[str, None], + more: list[str], + max_patterns: int) -> None: '''Add ``more`` to ``patterns`` (an ordered set), within - ``MAX_PATTERNS``. + ``max_patterns``. ''' patterns.update(dict.fromkeys(more)) - if len(patterns) > MAX_PATTERNS: + if len(patterns) > max_patterns: raise WorkspaceError( - f"more than {MAX_PATTERNS} sparse-checkout patterns (too many tests)" + f"more than {max_patterns} sparse-checkout patterns (too many tests; " + f"{ENV_PREFIX}MAX_SPARSE_PATTERNS)" ) @@ -293,9 +302,10 @@ def remove_tree(path: Path) -> None: class _Budget: '''What extracting an artifact may still spend: bytes, and entries.''' - def __init__(self, max_bytes: int) -> None: + def __init__(self, max_bytes: int, max_entries: int) -> None: self.bytes = max_bytes - self.entries = MAX_ARTIFACT_ENTRIES + self.max_entries = max_entries + self.entries = max_entries def spend(self, count: int) -> None: self.bytes -= count @@ -306,7 +316,8 @@ def count_entry(self) -> None: self.entries -= 1 if self.entries < 0: raise UnsafeArchiveError( - f"artifact has more than {MAX_ARTIFACT_ENTRIES} entries" + f"artifact has more than {self.max_entries} entries " + f"({ENV_PREFIX}MAX_ARTIFACT_ENTRIES)" ) @@ -353,24 +364,24 @@ def _check_inner_tarball(path: Path, budget: _Budget) -> None: raise UnsafeArchiveError(f"{path.name} is not a valid tarball") from ex -def extract_artifact(zip_path: Path, - destination: Path, - *, - max_bytes: int) -> None: +def extract_artifact( + zip_path: Path, destination: Path, *, max_bytes: int, + max_entries: int = DEFAULT_MAX_ARTIFACT_ENTRIES, + max_directory_bytes: int = DEFAULT_MAX_ARTIFACT_DIRECTORY_BYTES) -> None: '''Safely extract a CI artifact zip into the (new, empty) ``destination``. - Rejects absolute or escaping paths and symlinks, and stops once more than - ``max_bytes`` would be written, or ``MAX_ARTIFACT_ENTRIES`` entries made - (counting inner tarballs' contents too). + Rejects absolute or escaping paths and symlinks, and a zip whose central + directory is over ``max_directory_bytes``, and stops once more than + ``max_bytes`` would be written, or ``max_entries`` entries made (counting + inner tarballs' contents too). ''' directory_size = _central_directory_size(zip_path) - if (directory_size is not None and - directory_size > MAX_CENTRAL_DIRECTORY_BYTES): + if directory_size is not None and directory_size > max_directory_bytes: raise UnsafeArchiveError( - f"artifact's central directory is over " - f"{MAX_CENTRAL_DIRECTORY_BYTES} bytes" + f"artifact's central directory is over {max_directory_bytes} bytes " + f"({ENV_PREFIX}MAX_ARTIFACT_DIRECTORY_BYTES)" ) - budget = _Budget(max_bytes) + budget = _Budget(max_bytes, max_entries) tarballs = [] try: with zipfile.ZipFile(zip_path) as archive: @@ -406,11 +417,18 @@ def extract_artifact(zip_path: Path, class Workspaces: - '''Clones and worktrees under ``home`` for one forge.''' + '''Clones and worktrees under ``home`` for one forge; a diff (``diff``) + may be at most ``max_diff_bytes``. + ''' - def __init__(self, home: Path, forge: Forge) -> None: + def __init__(self, + home: Path, + forge: Forge, + *, + max_diff_bytes: int = DEFAULT_MAX_DIFF_BYTES) -> None: self._home = home self._forge = forge + self._max_diff_bytes = max_diff_bytes self._repos_dir = home / 'repos' / forge.name self._reviews_dir = home / REVIEWS_DIR self._locks: dict[str, asyncio.Lock] = {} @@ -597,7 +615,7 @@ async def diff(self, 'diff', '--no-renames', '--no-color', '--no-ext-diff', '--no-textconv', base, head, '--', *paths, git_dir = self.clone_dir(repository), - max_output = MAX_DIFF_BYTES + max_output = self._max_diff_bytes ) return result.stdout diff --git a/dev_tools/services/acknowledg-forge/README.md b/dev_tools/services/acknowledg-forge/README.md index fda34b5f75..1568da7961 100644 --- a/dev_tools/services/acknowledg-forge/README.md +++ b/dev_tools/services/acknowledg-forge/README.md @@ -112,6 +112,10 @@ from the environment variable shown as a fallback. | `ACKNOWLEDG_FORGE_SETTLE_TIMEOUT` | `7200` | Seconds after a push to stop waiting for CI before commenting | | `ACKNOWLEDG_FORGE_MAX_ARTIFACT_BYTES` | 256 MiB | Largest artifact download | | `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_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 | | `ACKNOWLEDG_FORGE_UNSANDBOXED` | `0` | `1` runs without containers: the service serves the runner servers and the publisher server itself, on `127.0.0.1`, where any local process can reach them. **Local development on a single-user machine only.** | diff --git a/dev_tools/services/acknowledg-forge/SANDBOXING.md b/dev_tools/services/acknowledg-forge/SANDBOXING.md index 0e2e2767d7..c89d1401a9 100644 --- a/dev_tools/services/acknowledg-forge/SANDBOXING.md +++ b/dev_tools/services/acknowledg-forge/SANDBOXING.md @@ -526,6 +526,9 @@ them. Several layers keep the service from reading through one: | Resource | Limit | | --- | --- | | 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 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 | | Each container's `/tmp` | 512 MiB tmpfs; copies made for diffing are streamed there | diff --git a/dev_tools/services/acknowledg-forge/tests/conftest.py b/dev_tools/services/acknowledg-forge/tests/conftest.py index af2cbaac91..d4b486a4a7 100644 --- a/dev_tools/services/acknowledg-forge/tests/conftest.py +++ b/dev_tools/services/acknowledg-forge/tests/conftest.py @@ -301,7 +301,11 @@ def __init__(self, builder.start() _builder_clients.append(builder) self.sandbox = Sandbox(builder = builder) - self.workspaces = Workspaces(config.home, forge) + self.workspaces = Workspaces( + config.home, + forge, + max_diff_bytes = config.max_diff_bytes + ) self.ingestor = Ingestor( store, forge, diff --git a/dev_tools/services/acknowledg-forge/tests/test_safety.py b/dev_tools/services/acknowledg-forge/tests/test_safety.py index f4f2a7e116..822538d76a 100644 --- a/dev_tools/services/acknowledg-forge/tests/test_safety.py +++ b/dev_tools/services/acknowledg-forge/tests/test_safety.py @@ -17,11 +17,10 @@ import pytest -from edgacknowledg.forge import processor, workspace +from edgacknowledg.forge import processor from edgacknowledg.forge.processor import _changes_test_ids, _read_ci_metadata from edgacknowledg.forge.sandbox import Sandbox, SandboxUnavailableError from edgacknowledg.forge.workspace import ( - MAX_PATTERNS, UnsafeArchiveError, WorkspaceError, _central_directory_size, @@ -90,14 +89,17 @@ def test_extract_caps_inner_tarballs(tmp_path: Path) -> None: extract_artifact(archive, destination, max_bytes = 10_000) -def test_extract_caps_entries(tmp_path: Path, - monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr(workspace, 'MAX_ARTIFACT_ENTRIES', 3) +def test_extract_caps_entries(tmp_path: Path) -> None: destination = tmp_path / 'out' destination.mkdir() archive = _zip(tmp_path / 'a.zip', {f"{n}.txt": b'' for n in range(4)}) with pytest.raises(UnsafeArchiveError, match = 'more than 3 entries'): - extract_artifact(archive, destination, max_bytes = 10_000) + extract_artifact( + archive, + destination, + max_bytes = 10_000, + max_entries = 3 + ) # Inner tarballs' entries count too. payload = io.BytesIO() @@ -111,22 +113,35 @@ def test_extract_caps_entries(tmp_path: Path, destination = tmp_path / 'out2' destination.mkdir() with pytest.raises(UnsafeArchiveError, match = 'more than 3 entries'): - extract_artifact(archive, destination, max_bytes = 10_000) + extract_artifact( + archive, + destination, + max_bytes = 10_000, + max_entries = 3 + ) -def test_extract_caps_the_central_directory( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch) -> None: +def test_extract_caps_the_central_directory(tmp_path: Path) -> None: # zipfile would read it all before the entries could be counted. archive = _zip(tmp_path / 'a.zip', {f"{n}.txt": b'' for n in range(10)}) size = _central_directory_size(archive) assert size is not None - monkeypatch.setattr(workspace, 'MAX_CENTRAL_DIRECTORY_BYTES', size - 1) destination = tmp_path / 'out' destination.mkdir() with pytest.raises(UnsafeArchiveError, match = 'central directory'): - extract_artifact(archive, destination, max_bytes = 10_000) + extract_artifact( + archive, + destination, + max_bytes = 10_000, + max_directory_bytes = size - 1 + ) assert list(destination.iterdir()) == [] + extract_artifact( + archive, + destination, + max_bytes = 10_000, + max_directory_bytes = size + ) def _zipfile_directory_size(path: Path) -> int: @@ -239,7 +254,8 @@ def expectations(suite: str) -> str: '+a.sft.cpp: PASS', ]) - assert '/tests/tests/cwg/a.sft.cpp' in changes_patterns(expectations('cwg')) + patterns = changes_patterns(expectations('cwg')) + assert '/tests/tests/cwg/a.sft.cpp' in patterns for suite in ['s' * 65, 'a.b', 'a b']: assert changes_patterns(expectations(suite)) == [ '/tests/.edgy/', '/tests/expectations/', @@ -247,16 +263,22 @@ def expectations(suite: str) -> str: def test_patterns_are_capped() -> None: + cap = 100 # Repeats aren't counted. - failures_patterns(['s/a.sft.cpp'] * (MAX_PATTERNS + 1)) + failures_patterns(['s/a.sft.cpp'] * (cap + 1), max_patterns = cap) with pytest.raises(WorkspaceError, match = 'sparse-checkout patterns'): - failures_patterns([f"s/{n}.sft.cpp" for n in range(MAX_PATTERNS)]) + failures_patterns( + [f"s/{n}.sft.cpp" for n in range(cap)], + max_patterns = cap + ) diff = '\n'.join([ 'diff --git a/tests/expectations/x/s.log b/tests/expectations/x/s.log', - *(f"+{n}.sft.cpp: FAIL" for n in range(MAX_PATTERNS)), + *(f"+{n}.sft.cpp: FAIL" for n in range(cap)), ]) with pytest.raises(WorkspaceError, match = 'sparse-checkout patterns'): - changes_patterns(diff) + changes_patterns(diff, max_patterns = cap) + # Within a higher cap. + changes_patterns(diff, max_patterns = 3 * cap) async def test_git_output_can_be_bounded() -> None: diff --git a/dev_tools/services/acknowledg-forge/tests/test_state.py b/dev_tools/services/acknowledg-forge/tests/test_state.py index ed69385110..2b45050ae7 100644 --- a/dev_tools/services/acknowledg-forge/tests/test_state.py +++ b/dev_tools/services/acknowledg-forge/tests/test_state.py @@ -426,6 +426,38 @@ def test_webhooks_need_the_service_bind(tmp_path: Path) -> None: make_config(tmp_path / 'home', WEBHOOK_BIND = '127.0.0.1:0') +@pytest.mark.parametrize('setting, problem', [ + ('MAX_DIFF_BYTES', "git diff's output is over 1 bytes"), + ('MAX_SPARSE_PATTERNS', 'MAX_SPARSE_PATTERNS'), + ('MAX_ARTIFACT_ENTRIES', 'MAX_ARTIFACT_ENTRIES'), + ('MAX_ARTIFACT_DIRECTORY_BYTES', 'MAX_ARTIFACT_DIRECTORY_BYTES'), +]) +async def test_limits_are_settings(tmp_path: Path, + origin: Origin, + forge: FakeForge, + store: ForgeStore, + clock: Clock, + setting: str, + problem: str) -> None: + # A pull request (with CI results) within the defaults... + config = make_config(tmp_path / 'home', **{setting: '1'}) + harness = Harness(store, forge, config, clock) + await harness.start() + head = origin.push_pull_request(1, PR_FILES) + forge.set_pull_request(pull_request_info(1, head, origin.main_sha)) + artifact = tmp_path / 'results.zip' + make_test_artifact(artifact, head, ['suite/b.sft.cpp']) + forge.artifact_files['a1'] = artifact + forge.runs[head] = ci_run(head, artifacts = ( + ArtifactInfo('test-results-all', 'a1', 1, None, False), + )) + await harness.step() + # ...is over the one set lowest. + record = await store.find_pull_request('fake', REPOSITORY, 1) + assert record is not None and record.last_error is not None + assert problem in record.last_error + + async def test_bench_sections_from_ci_artifact(tmp_path: Path, origin: Origin, forge: FakeForge,