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/bin/edg-rematerialize-test-run b/dev_tools/bin/edg-rematerialize-test-run index 4f2c99b7f1..d4c3948613 100755 --- a/dev_tools/bin/edg-rematerialize-test-run +++ b/dev_tools/bin/edg-rematerialize-test-run @@ -73,8 +73,14 @@ def build_diff_files(diff_file: Path, curr_diff_file_descr = None diff_file_to_handle = {} - for line in diff_file_handle: - line_match = GIT_DIFF_REGEX.match(line) + # Lines are read in bounded pieces so that an enormous one is never held + # whole; only a piece that starts a line can be a header, and every piece + # is written through unchanged. + for line, at_line_start in edgutil.iter_line_pieces( + diff_file_handle, + edgutil.MAX_LINE_CHARS + ): + line_match = GIT_DIFF_REGEX.match(line) if at_line_start else None if line_match is not None: if curr_diff_file_descr is not None: # Close the file handle for the previous diff file (if any) to @@ -180,8 +186,15 @@ def collect_affected_tests(test_home: Path, suite_name = file_path.stem # An expectations update. with open(diff_file.diff_file) as file_handle: - for line in file_handle: - if line.startswith('---') or line.startswith('+++'): + # edgutil.MAX_LINE_CHARS is far more than any genuine expectation + # entry needs, so a longer line is refused (as when the file is + # read), without being held whole. + try: + for line in edgutil.iter_limited_lines( + file_handle, + edgutil.MAX_LINE_CHARS + ): + if line.startswith('---') or line.startswith('+++'): # Do not take in lines like: # # --- a/tests/expectations/edg_x86_64_cp/modules.log @@ -189,13 +202,19 @@ def collect_affected_tests(test_home: Path, # continue - if line.startswith('-') or line.startswith('+'): - # Do a naive parse to get the test file name. - test_file_rel_path = PurePosixPath(line[1:].split(':')[0]) - result.tests.add( - tests_dir.joinpath(suite_name, test_file_rel_path) - ) - result.configs.add(config_name) + if line.startswith('-') or line.startswith('+'): + # Do a naive parse to get the test file name. + test_file_rel_path = PurePosixPath(line[1:].split(':')[0]) + result.tests.add( + tests_dir.joinpath(suite_name, test_file_rel_path) + ) + result.configs.add(config_name) + except edgutil.LineTooLongError: + eprint( + f"{diff_file.file_name}: a changed line is longer than " + f"{edgutil.MAX_LINE_CHARS} characters" + ) + sys.exit(1) return result @@ -548,10 +567,20 @@ async def simulate_test(test_home: Path, config_run_dir: Path, return test_status_lines def read_suite_expectation_lines(suite_file: Path) -> List[str]: + '''The lines of ``suite_file`` (none if there is no such file); exits if + one is longer than ``edgutil.MAX_LINE_CHARS``, rather than holding it. + ''' if not suite_file.exists(): return [] - with open(suite_file, 'r') as file_handle: - return file_handle.readlines() + try: + with open(suite_file, 'r') as file_handle: + return list(edgutil.iter_limited_lines( + file_handle, + edgutil.MAX_LINE_CHARS + )) + except edgutil.LineTooLongError as ex: + eprint(f"{suite_file}: {ex}") + sys.exit(1) def load_test_suite_expectations(all_expectations: Dict[str, List[str]], suite_name: str, expectations_dir: Path, diff --git a/dev_tools/pylibs/edgacknowledg/__init__.py b/dev_tools/pylibs/edgacknowledg/__init__.py index 61697fc2f7..403706faed 100644 --- a/dev_tools/pylibs/edgacknowledg/__init__.py +++ b/dev_tools/pylibs/edgacknowledg/__init__.py @@ -121,12 +121,17 @@ def request_tags_with_prefix(prefix: str) -> FrozenSet[str]: ALLOWED_BENCH_EDIT_TAGS ) -PROTOCOL_REVISION = 19 +PROTOCOL_REVISION = 20 # 10MiB MAX_MESSAGE_SIZE = 10485760 # 8MiB MAX_DISPLAY_CONTENT_SIZE = 8388608 +# 16Ki characters: a longer line is cut for display (see lines_truncated). +MAX_DISPLAY_LINE_SIZE = 16384 +# 1Ki characters: a longer label (e.g. a status line's variant, at most a few +# hundred in practice) is cut for display, ending in an ellipsis. +MAX_DISPLAY_LABEL_SIZE = 1024 WebSocketsMessageType = Union[str, bytes] ProtocolMessage = Dict[str, Any] diff --git a/dev_tools/pylibs/edgacknowledg/client/shared.py b/dev_tools/pylibs/edgacknowledg/client/shared.py index 675f781cc1..ad2a243f7c 100644 --- a/dev_tools/pylibs/edgacknowledg/client/shared.py +++ b/dev_tools/pylibs/edgacknowledg/client/shared.py @@ -7,6 +7,7 @@ import asyncio import errno +import io import os import signal import stat @@ -19,6 +20,7 @@ Dict, List, IO, + Iterable, Iterator, NamedTuple, Optional, @@ -37,6 +39,8 @@ from edgacknowledg import ( MAX_DISPLAY_CONTENT_SIZE, + MAX_DISPLAY_LABEL_SIZE, + MAX_DISPLAY_LINE_SIZE, ProtocolMessage, RequestTags, RichName, @@ -83,6 +87,8 @@ async def handle_request(self, channel: Channel) -> None: ReviewClientHandler = Any # type: ignore[misc,assignment] +import edgutil + from edgutil import get_running_event_loop, is_path_relative_to CoroFactory = Callable[[], Awaitable[Any]] @@ -149,32 +155,111 @@ async def announce_section(channel: Channel, )) -def trunc_display_content(content: str) -> Dict[str, Union[str, bool]]: - '''Truncate ``content`` for UI display; may set ``truncated``.''' - max_content_size = MAX_DISPLAY_CONTENT_SIZE - if len(content) > max_content_size: - # Attempt to truncate at line boundaries. - # - # To save time, find the newline closest to the boundary before walking - # the file size back. - last_newline_permitted = content.find('\n', max_content_size) - if last_newline_permitted != -1: - content = content[:last_newline_permitted] - # Now walk the file size back. - while len(content) > max_content_size: - last_new_line_idx = content.rfind('\n') - if last_new_line_idx != -1: - content = content[:(last_new_line_idx - 1)] - else: - # If there are no more newlines, resort to truncating within - # the line. - content = content[:(max_content_size - 4)] - content += '...' - return { - 'truncated': True, - 'content': content - } - return { 'content': content } +def trunc_display_label(text: str) -> str: + '''``text`` (one line, e.g. a status) cut for UI display to + ``MAX_DISPLAY_LABEL_SIZE`` characters, a cut ending in an ellipsis. + ''' + if len(text) > MAX_DISPLAY_LABEL_SIZE: + text = text[:MAX_DISPLAY_LABEL_SIZE - 1] + '\N{HORIZONTAL ELLIPSIS}' + return text + + +def _display_lines( + pieces: Iterable[Tuple[str, bool]]) -> Iterator[Tuple[str, str, bool]]: + '''Join ``(piece, at_line_start)`` pieces (as ``edgutil.iter_line_pieces`` + yields them) into lines, yielding ``(text, line_end, cut)`` for each: at + most ``MAX_DISPLAY_LINE_SIZE`` characters of its text (``cut`` if it had + more, which are dropped as they come), and its end ('\\n', or '' for a + last line without one). + + A piece that starts a line ends the one before it, so a text that doesn't + end in a line end (e.g. a command's output) never runs into the next. + ''' + # The line being joined (as much of it as is kept), and whether some was + # dropped; ``None`` between lines. + line: Optional[str] = None + cut = False + for piece, at_line_start in pieces: + if line is not None and at_line_start: + yield line, '\n', cut + line = None + if line is None: + line = '' + cut = False + text = piece[:-1] if piece.endswith('\n') else piece + room = MAX_DISPLAY_LINE_SIZE - len(line) + if len(text) > room: + cut = True + line += text[:room] + if piece.endswith('\n'): + yield line, '\n', cut + line = None + if line is not None: + yield line, '', cut + + +def display_content(pieces: Iterable[Tuple[str, bool]]) -> Dict[str, Any]: + '''The ``content`` of a display payload, from ``(piece, at_line_start)`` + pieces (as ``edgutil.iter_line_pieces`` yields them), and as needed + ``truncated`` and ``lines_truncated``: + + - Each line is cut to ``MAX_DISPLAY_LINE_SIZE`` characters, its 1-based + number listed in ``lines_truncated``. + - The whole is cut to ``MAX_DISPLAY_CONTENT_SIZE``, after the last line + that fits (setting ``truncated``); no more pieces are taken then, so + what they come from is read no further. (Closing ``pieces``, should it + be a generator with files open, is left to the caller.) + + Neither key is set when nothing was cut, and nothing marks a cut in the + content itself: the UI does. + ''' + kept: List[str] = [] + size = 0 + lines_truncated: List[int] = [] + truncated = False + for line_number, (text, line_end, cut) in enumerate( + _display_lines(pieces), + start = 1 + ): + num_chars_for_line = len(text) + len(line_end) + if size + num_chars_for_line > MAX_DISPLAY_CONTENT_SIZE: + truncated = True + if not kept: + # The first line alone doesn't fit (MAX_DISPLAY_LINE_SIZE is + # larger than MAX_DISPLAY_CONTENT_SIZE): keep what does of it, as + # a cut line. + kept.append(text[:MAX_DISPLAY_CONTENT_SIZE]) + lines_truncated.append(line_number) + break + kept.append(text + line_end) + size += num_chars_for_line + if cut: + lines_truncated.append(line_number) + + content = ''.join(kept) + if truncated and content.endswith('\n'): + # Without the end of the last line kept. + content = content[:-1] + result: Dict[str, Any] = { 'content': content } + if truncated: + result['truncated'] = True + if lines_truncated: + result['lines_truncated'] = lines_truncated + return result + + +def display_text(handle: IO[str]) -> Dict[str, Any]: + '''``display_content`` of the text ``handle`` reads: only as much as is + shown, and of a line, at most a character more. + ''' + return display_content( + edgutil.iter_line_pieces(handle, MAX_DISPLAY_LINE_SIZE + 1) + ) + + +def trunc_display_content(content: str) -> Dict[str, Any]: + '''``display_content`` of ``content``, already in memory.''' + return display_text(io.StringIO(content)) def reviewer_of(request: Any) -> Optional[str]: diff --git a/dev_tools/pylibs/edgacknowledg/client/test/__init__.py b/dev_tools/pylibs/edgacknowledg/client/test/__init__.py index c6a40c1b55..827df4a015 100644 --- a/dev_tools/pylibs/edgacknowledg/client/test/__init__.py +++ b/dev_tools/pylibs/edgacknowledg/client/test/__init__.py @@ -6,6 +6,9 @@ '''AcknowlEDG test-review provider package.''' import asyncio +import contextlib +import filecmp +import functools import os import re import shutil @@ -27,7 +30,7 @@ ) from edgacknowledg import ( - MAX_DISPLAY_CONTENT_SIZE, + MAX_DISPLAY_LINE_SIZE, ProtocolMessage, RequestTags, WebSocketsMessageType, @@ -40,8 +43,11 @@ ConfinedPath, IllegalFileAccessError, SandboxContext, + display_content, + display_text, reviewer_of, trunc_display_content, + trunc_display_label, ) from edgutil import eprint @@ -70,7 +76,13 @@ def _create_status_dict(elog_file_paths: List[ConfinedPath], *, for elog_file_path in elog_file_paths: config_name = elog_file_path.parent.name with elog_file_path.open() as file_handle: - for line in file_handle: + # edgutil.MAX_LINE_CHARS is far more than any genuine status line + # needs, so a longer line is refused (failing the request, as a line + # that isn't a status line does), without being held whole. + for line in edgutil.iter_limited_lines( + file_handle, + edgutil.MAX_LINE_CHARS + ): key, parsed_diff = ( edgtest.runtest.parse_test_status_diff(line) ) @@ -79,7 +91,7 @@ def _create_status_dict(elog_file_paths: List[ConfinedPath], *, pending[key] = {} lines_by_test_config[key] = {} if parsed_diff.remark: - remarks[key] = parsed_diff.remark + remarks[key] = trunc_display_label(parsed_diff.remark) if config_name not in pending[key]: rt_file_path = _rt_path_for(elog_file_path.parent, key) @@ -89,9 +101,9 @@ def _create_status_dict(elog_file_paths: List[ConfinedPath], *, lines_by_test_config[key][config_name] = [] lines_by_test_config[key][config_name].append({ - "qualifier": parsed_diff.qualifier, - "variant": parsed_diff.variant, - "status": parsed_diff.status.full_status + "qualifier": trunc_display_label(parsed_diff.qualifier), + "variant": trunc_display_label(parsed_diff.variant), + "status": trunc_display_label(parsed_diff.status.full_status) }) keyed_statuses: _StatusDict = {} @@ -286,13 +298,17 @@ def test_id_to_path(test_id: str) -> ConfinedPath: )) return - async def form_curr_diff_text(config_name: str, - case_rt_path: ConfinedPath) -> str: - '''Live-diff observed vs recordings per command. - - Per-command body content is diffed (recording command line stripped), - then equal bodies are collapsed with # command comments — same - presentation as static. + async def write_curr_diff(config_name: str, + case_rt_path: ConfinedPath, + out_path: Path) -> None: + '''Write the live diff of observed output vs recordings to + ``out_path``. + + Per-command body content is diffed (recording command line + stripped), then equal bodies are collapsed with # command + comments, the same presentation as static. The per-command diffs + are files of its own, gone once ``out_path`` is written; none is + held in memory. ''' suite_name = test_id.split('/')[0] suite_path = test_home / 'tests' / suite_name @@ -311,8 +327,7 @@ async def form_curr_diff_text(config_name: str, with tempfile.TemporaryDirectory() as tmp_dir_name: tmp_dir = Path(tmp_dir_name) - entries: List[Tuple[Dict[str, Any], List[str]]] = [] - + entries: List[Tuple[Dict[str, Any], Path]] = [] with edgtest.runtest.RunTestOutput(case_rt_path.path) as rt_output: for case_number, case in enumerate( rt_output.metadata.get('cases', []), @@ -355,13 +370,9 @@ async def form_curr_diff_text(config_name: str, if diff_hash is None: continue - with open(change_diff_path, 'r', - errors = 'replace') as file_handle: - diff_lines = file_handle.readlines() - command_entry = dict(command) command_entry['diff_hash'] = diff_hash - entries.append((command_entry, diff_lines)) + entries.append((command_entry, change_diff_path)) if case.get('matches') is None: continue @@ -395,57 +406,80 @@ async def form_curr_diff_text(config_name: str, if matches_diff_hash is None: continue - with open(matches_change_diff, 'r', - errors = 'replace') as file_handle: - matches_diff_lines = file_handle.readlines() - matches_entry = { 'command': edgtest.runtest.case_matches_annotation( case_number ), 'diff_hash': matches_diff_hash } - entries.append((matches_entry, matches_diff_lines)) - - return ''.join( - edgtest.runtest.format_collapsed_case_diffs(entries) - ) + entries.append((matches_entry, matches_change_diff)) + + with open(out_path, 'w', encoding = 'utf-8') as out_handle: + with contextlib.closing( + edgtest.runtest.iter_collapsed_case_diff_pieces( + [ + (entry, + functools.partial(open, path, 'r', errors = 'replace')) + for entry, path in entries + ], + edgutil.MAX_LINE_CHARS + ) + ) as pieces: + for piece, _ in pieces: + out_handle.write(piece) + # What is shown is read only as far as it is shown: never a whole + # member of the run artifact, or a whole line of one. + piece_limit = MAX_DISPLAY_LINE_SIZE + 1 if request_tag == RequestTags.TEST_SHOW_DIFF: with edgtest.runtest.RunTestOutput(rt_file_path.path) as rt_output: - static_text = ''.join(rt_output.combined_static_diff()) + with contextlib.closing( + rt_output.iter_static_diff_pieces(piece_limit) + ) as pieces: + shown = display_content(pieces) client_response = { 'id': configured_test_id, - **trunc_display_content(static_text) + **shown } elif request_tag == RequestTags.TEST_SHOW_RAW_OUTPUT: with edgtest.runtest.RunTestOutput(rt_file_path.path) as rt_output: - raw_output = ''.join(rt_output.combined_raw_output()) + with contextlib.closing( + rt_output.iter_raw_output_pieces(piece_limit) + ) as pieces: + shown = display_content(pieces) client_response = { 'id': configured_test_id, - **trunc_display_content(raw_output) + **shown } elif request_tag == RequestTags.TEST_SHOW_CURR_DIFF: rep_config = changes_file_path.parent.name group_configs = request_value.get('configs') or [rep_config] - rep_curr_text = await form_curr_diff_text(rep_config, rt_file_path) divergent_configs: List[str] = [] - for config_name in group_configs: - if config_name == rep_config: - continue - sibling_changes = changes_files[config_name] - sibling_rt = _rt_path_for(sibling_changes.parent, test_id) - if not sibling_rt.exists(): - divergent_configs.append(config_name) - continue - sibling_text = await form_curr_diff_text(config_name, sibling_rt) - if sibling_text != rep_curr_text: - divergent_configs.append(config_name) + with tempfile.TemporaryDirectory() as tmp_dir_name: + rep_path = Path(tmp_dir_name, 'curr.diff') + await write_curr_diff(rep_config, rt_file_path, rep_path) + for config_name in group_configs: + if config_name == rep_config: + continue + sibling_changes = changes_files[config_name] + sibling_rt = _rt_path_for(sibling_changes.parent, test_id) + if not sibling_rt.exists(): + divergent_configs.append(config_name) + continue + # Compared a chunk at a time, then gone. + sibling_path = Path(tmp_dir_name, 'sibling-curr.diff') + await write_curr_diff(config_name, sibling_rt, sibling_path) + if not filecmp.cmp(rep_path, sibling_path, shallow = False): + divergent_configs.append(config_name) + sibling_path.unlink() + + with open(rep_path, encoding = 'utf-8') as curr_diff_handle: + shown = display_text(curr_diff_handle) client_response = { 'id': configured_test_id, - **trunc_display_content(rep_curr_text) + **shown } if len(divergent_configs) != 0: @@ -549,10 +583,9 @@ async def form_curr_diff_text(config_name: str, } )) else: - # Only as much as can be shown is read (one character more marks - # the content as truncated). + # Only as much as can be shown is read. with path_to_show.open(errors = 'replace') as file_handle: - shown = file_handle.read(MAX_DISPLAY_CONTENT_SIZE + 1) + shown = display_text(file_handle) await channel.send(message( RequestTags.TEST_SHOW_SOURCE, value = { @@ -560,7 +593,7 @@ async def form_curr_diff_text(config_name: str, 'client_response': { 'id': test_id, 'assoc_file': request_value.get('assoc_file'), - **trunc_display_content(shown) + **shown } } )) 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/http.py b/dev_tools/pylibs/edgacknowledg/forge/http.py index 1398076465..7a905c86d5 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/http.py +++ b/dev_tools/pylibs/edgacknowledg/forge/http.py @@ -26,6 +26,9 @@ _log = logging.getLogger(__name__) PAGE_SIZE = 100 +# The largest API response read (decoded): far more than a page of +# PAGE_SIZE items; downloads have their own limit. +MAX_RESPONSE_BYTES = 32 * 1024 * 1024 MAX_RATE_LIMIT_WAIT = 15 * 60.0 MAX_ATTEMPTS = 4 @@ -88,17 +91,19 @@ async def request(self, params: Optional[Mapping[str, Any]] = None, json_body: Optional[Any] = None) -> httpx.Response: '''Send a request, retrying transient failures and waiting out rate - limits; raise ``ForgeError`` on failure. + limits; raise ``ForgeError`` on failure, or for a response over + ``MAX_RESPONSE_BYTES`` (read only that far). ''' for attempt in range(MAX_ATTEMPTS): last_attempt = attempt + 1 == MAX_ATTEMPTS try: - response = await self._client.request( + request = self._client.build_request( method, path, params = params, json = json_body ) + response = await _send_capped(self._client, request, method, path) except httpx.HTTPError as ex: if last_attempt: raise ForgeError(f"{method} {path} failed: {ex}") from ex @@ -179,6 +184,40 @@ async def download(self, raise ForgeError(f"download of {path} failed: {ex}") from ex +async def _send_capped(client: httpx.AsyncClient, + request: httpx.Request, + method: str, + path: str) -> httpx.Response: + '''Send ``request``; return its response, its body read (decoded) as it + comes, and only to ``MAX_RESPONSE_BYTES``: ``ForgeError`` beyond. + ''' + response = await client.send(request, stream = True) + try: + body = bytearray() + async for chunk in response.aiter_bytes(): + body += chunk + if len(body) > MAX_RESPONSE_BYTES: + raise ForgeError( + f"{method} {path}: the response is over {MAX_RESPONSE_BYTES} bytes" + ) + finally: + await response.aclose() + # The same response with its body in place; it is decoded already, so + # without the headers that describe how it was sent. + headers = [ + (name, value) for name, value in response.headers.multi_items() + if name.lower() not in ('content-encoding', 'content-length', + 'transfer-encoding') + ] + return httpx.Response( + response.status_code, + headers = headers, + content = bytes(body), + request = request, + extensions = response.extensions + ) + + async def _write_capped(stream: httpx.Response, destination: Path, max_bytes: int, diff --git a/dev_tools/pylibs/edgacknowledg/forge/processor.py b/dev_tools/pylibs/edgacknowledg/forge/processor.py index ac17bc9405..3dbf594592 100644 --- a/dev_tools/pylibs/edgacknowledg/forge/processor.py +++ b/dev_tools/pylibs/edgacknowledg/forge/processor.py @@ -76,6 +76,9 @@ # message. MAX_CI_METADATA_BYTES = 64 * 1024 MAX_TEST_IDS_BYTES = 4 * 1024 * 1024 +# The longest test id taken from CI (a path within a suite); a longer line +# names no test, and is skipped rather than carried along. +MAX_TEST_ID_CHARS = 4096 # Retries for a failing pull request: 1 min, 2 min, ... up to 1 h; give up # after this many attempts until the pull request changes again. @@ -117,7 +120,8 @@ def _reusable(section: Optional[SectionRecord], def _changes_test_ids(changes_elists: Iterable[Path]) -> list[str]: '''The test ids ``changes_elists`` name, within ``MAX_TEST_IDS_BYTES`` - (together, both as read and as JSON); ``WorkspaceError`` beyond. + (together, both as read and as JSON); ``WorkspaceError`` beyond. A line + longer than ``MAX_TEST_ID_CHARS`` is skipped. ''' test_ids = [] remaining = MAX_TEST_IDS_BYTES @@ -130,7 +134,14 @@ def _changes_test_ids(changes_elists: Iterable[Path]) -> list[str]: f"CI's changes.elist files are over {MAX_TEST_IDS_BYTES} bytes" ) text = content.decode('utf-8', errors = 'replace') - test_ids += [line.strip() for line in text.splitlines() if line.strip()] + for line in text.splitlines(): + if len(line) > MAX_TEST_ID_CHARS: + _log.warning('skipped a changes.elist line of %d characters', + len(line)) + continue + test_id = line.strip() + if test_id: + test_ids.append(test_id) # JSON escapes control and non-ASCII characters (e.g. the U+FFFD of bytes # that aren't UTF-8), up to six bytes each. if len(json.dumps(test_ids)) > MAX_TEST_IDS_BYTES: @@ -397,42 +408,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 +500,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 +511,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..9c675ab37a 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 IO, Callable, Iterable, Optional, Sequence import edgtest import edgutil @@ -72,6 +73,14 @@ # files) before it is killed; well within docker stop's default 10. _TERMINATE_ALLOWANCE = 5.0 +# The most output a git command run for its result (not one whose output +# goes to a sink) may have; far more than a rev-parse or for-each-ref of a +# pull request's refs has. +_MAX_GIT_OUTPUT_BYTES = 16 * 1024 * 1024 +# How much of a git command's stderr is kept (the rest is read, and +# dropped): a fetch relays what the remote says, which needn't be short. +_MAX_GIT_STDERR_BYTES = 64 * 1024 + class WorkspaceError(Exception): '''Raised when a workspace operation fails or input is unsafe.''' @@ -81,6 +90,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 @@ -130,26 +143,26 @@ def _patterns_for_changed_test_file(path: PurePosixPath) -> list[str]: return [f"/{_escape_pattern(str(path.parent))}/"] -def changed_paths(diff: str) -> list[str]: - '''Return the paths a ``git diff --no-renames`` output touches.''' - paths = [] - for line in diff.splitlines(): - match = _DIFF_HEADER.match(line) - if match is not None: - paths.append(match.group(1)) - return paths - - -def changes_patterns(diff: str, +def changes_patterns(diff: IO[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 - ``max_patterns``. + '''Sparse-checkout patterns for rematerializing the diff (of tests/) read + from ``diff`` 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(): + limit = edgutil.MAX_LINE_CHARS + for piece, at_line_start in edgutil.iter_line_pieces(diff, limit + 1): + # Only whole lines within edgutil.MAX_LINE_CHARS matter: no header or + # expectation line is longer, and a longer one (e.g. of a recording) is + # passed over in pieces, never held whole. + if not at_line_start: + continue + text_length = len(piece) - (1 if piece.endswith('\n') else 0) + if text_length > limit: + continue + line = piece.rstrip('\n') match = _DIFF_HEADER.match(line) if match is not None: path = PurePosixPath(match.group(1)) @@ -478,9 +491,15 @@ 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). Without, its output goes into the result, + and may be at most ``_MAX_GIT_OUTPUT_BYTES`` (``WorkspaceError`` + beyond). Only the start of its stderr is kept. ''' command = ['git'] if git_dir is not None: @@ -497,18 +516,31 @@ async def git(self, cwd = cwd, env = self._git_env() ) - if 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" + stdin_bytes = stdin.encode() if stdin is not None else None + if stdout_sink is None: + assert max_output is None + collected = bytearray() + try: + stderr = await _communicate_bounded( + process, + _MAX_GIT_OUTPUT_BYTES, + collected.extend, + stdin_bytes ) - stdout, stderr = bounded + except _OutputTooLarge: + raise WorkspaceError( + f"git {' '.join(args)} wrote over {_MAX_GIT_OUTPUT_BYTES} bytes" + ) from None + stdout = bytes(collected) + else: + assert max_output is not None + stderr = await _communicate_bounded( + process, + max_output, + stdout_sink, + stdin_bytes + ) + stdout = b'' assert process.returncode is not None result = GitResult( process.returncode, @@ -609,15 +641,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, @@ -701,32 +760,68 @@ async def collect_garbage(self, repository: str, open_count: int) -> None: await self.git('gc', '--auto', '--quiet', git_dir = clone_dir) +async def _read_start(stream: asyncio.StreamReader, keep: int) -> bytes: + '''Read ``stream`` to its end; return its first ``keep`` bytes (the rest + is dropped as it comes). + ''' + kept = bytearray() + while chunk := await stream.read(64 * 1024): + kept += chunk[:max(0, keep - len(kept))] + return bytes(kept) + + +async def _write_stdin(process: asyncio.subprocess.Process, + data: bytes) -> None: + '''Write ``data`` to ``process``'s stdin, then close it.''' + assert process.stdin is not None + try: + process.stdin.write(data) + await process.stdin.drain() + except (BrokenPipeError, ConnectionResetError): + # It exited without reading it all; its exit status tells. + pass + finally: + process.stdin.close() + + 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], + stdin: Optional[bytes] = None) -> bytes: + '''Pass ``process``'s stdout to ``sink`` as it comes, in chunks (none held + here), having written ``stdin`` (if any) to it; return the start of its + stderr (``_MAX_GIT_STDERR_BYTES``) 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( + _read_start(process.stderr, _MAX_GIT_STDERR_BYTES) + ) + writing_stdin = ( + asyncio.create_task(_write_stdin(process, stdin)) + if stdin is not None else None + ) 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) + if writing_stdin is not None: + await writing_stdin + 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() + if writing_stdin is not None: + writing_stdin.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/pylibs/edgtest/runtest/__init__.py b/dev_tools/pylibs/edgtest/runtest/__init__.py index 0308d4b651..ff0002bdb2 100644 --- a/dev_tools/pylibs/edgtest/runtest/__init__.py +++ b/dev_tools/pylibs/edgtest/runtest/__init__.py @@ -12,6 +12,7 @@ import functools import hashlib import io +import itertools import json import os import re @@ -27,8 +28,8 @@ from collections import OrderedDict from pathlib import Path, PurePath from typing import ( - Any, Callable, Dict, IO, Iterator, List, Mapping, NamedTuple, Optional, - Pattern, Set, Tuple, cast + Any, Callable, Dict, Generator, IO, Iterator, List, Mapping, NamedTuple, + Optional, Pattern, Set, Tuple, cast ) def get_default_config_name(env: Optional[Dict[str, str]] = None) -> str: @@ -1195,6 +1196,59 @@ def emit_one(commands: List[Dict[str, Any]], result.extend(emit_one(group['commands'], group['diff_lines'])) return result +# A text that is opened when (and if) it is read. +TextOpener = Callable[[], IO[str]] + +def _hash_text_stream(open_text: TextOpener) -> str: + '''``hash_diff_text`` of the text ``open_text`` opens, read in chunks.''' + digest = hashlib.sha256() + with open_text() as handle: + while True: + chunk = handle.read(64 * 1024) + if not chunk: + break + digest.update(chunk.encode('utf-8', errors = 'replace')) + return digest.hexdigest() + +def iter_collapsed_case_diff_pieces( + command_diff_entries: List[Tuple[Dict[str, Any], TextOpener]], + limit: int) -> Generator[Tuple[str, bool], None, None]: + '''The collapsed ``format_collapsed_case_diffs`` of + ``command_diff_entries`` (with their stored labels), whose diffs are + opened rather than given as lines, as ``edgutil.iter_line_pieces`` pieces + of at most ``limit`` characters (each ``# command`` comment is one piece, + however long). A diff is read only as far as the pieces are taken, so + neither a long line nor a long diff is held whole. + ''' + # First-seen order of unique diff bodies (keyed by stored hash). + groups: Dict[str, Tuple[List[Dict[str, Any]], TextOpener]] = ( + OrderedDict() + ) + for command, open_text in command_diff_entries: + group_key = command.get('diff_hash') or _hash_text_stream(open_text) + if group_key not in groups: + groups[group_key] = ([], open_text) + groups[group_key][0].append(command) + + for commands, open_text in groups.values(): + with open_text() as handle: + pieces = edgutil.iter_line_pieces(handle, limit) + # The ---/+++ header, if it has one, comes before the comments; its + # lines are short (labels), so each must be one piece: a diff whose + # first lines are longer has no header (and isn't held to look). + head = list(itertools.islice(pieces, 2)) + if (len(head) == 2 and + all(at_line_start and piece.endswith('\n') + for piece, at_line_start in head) and + head[0][0].startswith('---') and + head[1][0].startswith('+++')): + yield from head + head = [] + for command in commands: + yield f"# {case_command_from_metadata(command)}\n", True + yield from head + yield from pieces + class RtTarContents: '''Manifest of filesystem files to pack into a .rt.tar[.gz]. @@ -1349,6 +1403,16 @@ def _require_tar(self) -> tarfile.TarFile: def _read_lines(self, member_name: str) -> List[str]: return _read_tar_lines_member(self._require_tar(), member_name) + def open_text_member(self, member_name: str) -> IO[str]: + '''Open a member as text (UTF-8, errors replaced), read as it is + decompressed: nothing is held but what is read. + ''' + return io.TextIOWrapper( + _extract_tar_member(self._require_tar(), member_name), + encoding = 'utf-8', + errors = 'replace' + ) + @property def metadata(self) -> Dict[str, Any]: if self._metadata is None: @@ -1430,9 +1494,9 @@ def iter_command_diffs(self) -> Iterator[List[str]]: for _entry, diff_lines in self.iter_command_diff_entries(): yield diff_lines - def iter_command_diff_entries( - self) -> Iterator[Tuple[Dict[str, Any], List[str]]]: - '''Yield (annotation metadata, change.diff lines) for diffs. + def iter_command_diff_members( + self) -> Iterator[Tuple[Dict[str, Any], str]]: + '''Yield (annotation metadata, change.diff member name) for diffs. Command diffs use the stored command metadata. Matches diffs use a synthetic command string ``Matches for case N``. @@ -1443,21 +1507,29 @@ def iter_command_diff_entries( case.get('commands', []), start = 1 ): - diff_lines = self.command_diff(case_number, command_number) - if diff_lines is not None: - yield command, diff_lines + member = f"cases/{case_number}/{command_number}/change.diff" + if member in self.member_names: + yield command, member matches = case.get('matches') if matches is None: continue - matches_diff_lines = self.case_matches_diff(case_number) - if matches_diff_lines is None: + member = f"cases/{case_number}/matches/change.diff" + if member not in self.member_names: continue matches_entry = { 'command': case_matches_annotation(case_number), 'diff_hash': matches.get('diff_hash') } - yield matches_entry, matches_diff_lines + yield matches_entry, member + + def iter_command_diff_entries( + self) -> Iterator[Tuple[Dict[str, Any], List[str]]]: + '''Yield (annotation metadata, change.diff lines) for diffs (see + ``iter_command_diff_members``). + ''' + for entry, member in self.iter_command_diff_members(): + yield entry, self._read_lines(member) def combined_static_diff(self, *, @@ -1475,6 +1547,48 @@ def combined_static_diff(self, collapse = collapse ) + def iter_static_diff_pieces( + self, + limit: int) -> Generator[Tuple[str, bool], None, None]: + '''``combined_static_diff()`` (collapsed, with the stored labels) as + ``edgutil.iter_line_pieces`` pieces of at most ``limit`` characters, + each member read only as far as the pieces are taken. + ''' + return iter_collapsed_case_diff_pieces( + [ + (entry, functools.partial(self.open_text_member, member)) + for entry, member in self.iter_command_diff_members() + ], + limit + ) + + def iter_raw_output_pieces( + self, + limit: int) -> Generator[Tuple[str, bool], None, None]: + '''``combined_raw_output()`` as ``edgutil.iter_line_pieces`` pieces of + at most ``limit`` characters (each ``# `` header line is one piece, + however long), each member read only as far as the pieces are taken. + ''' + for case_number, case in enumerate(self.metadata.get('cases', []), + start = 1): + for command_number, command in enumerate( + case.get('commands', []), + start = 1 + ): + command_str = case_command_from_metadata(command) + if command_str: + yield f"# {command_str}\n", True + with self.open_text_member( + f"cases/{case_number}/{command_number}/output.txt" + ) as handle: + yield from edgutil.iter_line_pieces(handle, limit) + + matches_member = f"cases/{case_number}/matches/output.txt" + if matches_member in self.member_names: + yield f"# {case_matches_annotation(case_number)}\n", True + with self.open_text_member(matches_member) as handle: + yield from edgutil.iter_line_pieces(handle, limit) + def combined_raw_output(self) -> List[str]: '''Concatenate command lines from metadata with each command's body. diff --git a/dev_tools/pylibs/edgutil/__init__.py b/dev_tools/pylibs/edgutil/__init__.py index e2dba357ea..a817d13729 100644 --- a/dev_tools/pylibs/edgutil/__init__.py +++ b/dev_tools/pylibs/edgutil/__init__.py @@ -27,7 +27,9 @@ Callable, Coroutine, Dict, + IO, Iterable, + Iterator, List, Optional, Set, @@ -193,6 +195,53 @@ def __exit__(self, def eprint(msg: str) -> None: print(msg, file = sys.stderr) +def iter_line_pieces(handle: IO[str], + limit: int) -> Iterator[Tuple[str, bool]]: + '''Yield ``(piece, at_line_start)`` for the text ``handle``, a line at a + time but never holding more than ``limit`` characters of one: a longer + line comes as several pieces, only the first of which has + ``at_line_start`` set. Concatenating the pieces gives back the text + exactly. + ''' + at_line_start = True + while True: + piece = handle.readline(limit) + if not piece: + break + yield piece, at_line_start + at_line_start = piece.endswith('\n') + +class LineTooLongError(ValueError): + '''Raised by ``iter_limited_lines`` for a line over its limit.''' + + def __init__(self, line_number: int, limit: int) -> None: + super().__init__( + f"line {line_number} is longer than {limit} characters" + ) + self.line_number = line_number + self.limit = limit + +def iter_limited_lines(handle: IO[str], limit: int) -> Iterator[str]: + '''Yield each line of the text ``handle``, with its line end ('\\n') if + it has one; raise ``LineTooLongError`` for a line longer than ``limit`` + characters (its end not counted), having read only ``limit + 1`` of + them. + ''' + # Every piece up to an overlong line's first is a whole line. + for line_number, (line, _) in enumerate( + iter_line_pieces(handle, limit + 1), + start = 1 + ): + text_length = len(line) - (1 if line.endswith('\n') else 0) + if text_length > limit: + raise LineTooLongError(line_number, limit) + yield line + +# The longest line a reader of untrusted line-oriented text (a pull +# request's diff, a CI artifact's files) need hold: far more than any path, +# test name, or status line, and small enough not to matter in memory. +MAX_LINE_CHARS = 64 * 1024 + def indent_text_block(text_block: str, n_times: int = 1) -> str: indent = ' ' * n_times return re.sub(r'^(.)', f"{indent}\\1", text_block, flags = re.MULTILINE) 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_display.py b/dev_tools/services/acknowledg-forge/tests/test_display.py new file mode 100644 index 0000000000..f4e43c2fc6 --- /dev/null +++ b/dev_tools/services/acknowledg-forge/tests/test_display.py @@ -0,0 +1,127 @@ +# 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 + +'''Content cut for display to reviewers: whole, and line by line.''' + +import contextlib +import time + +from typing import Iterator + +import pytest + +from edgacknowledg import MAX_DISPLAY_LABEL_SIZE +from edgacknowledg.client import shared +from edgacknowledg.client.shared import ( + display_content, + trunc_display_content, + trunc_display_label, +) + + +@pytest.fixture +def small_limits(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(shared, 'MAX_DISPLAY_CONTENT_SIZE', 40) + monkeypatch.setattr(shared, 'MAX_DISPLAY_LINE_SIZE', 8) + + +def test_short_content_is_untouched(small_limits: None) -> None: + for content in ('', 'a', 'a\nb\n', 'a\n\n\nb', '12345678\n'): + assert trunc_display_content(content) == {'content': content} + + +def test_long_lines_are_cut_and_numbered(small_limits: None) -> None: + result = trunc_display_content('short\n123456789\nok\n' + 'x' * 20) + assert result == { + 'content': 'short\n12345678\nok\n' + 'x' * 8, + 'lines_truncated': [2, 4], + } + + +def test_long_content_is_cut_at_a_line_end(small_limits: None) -> None: + # Eight lines of six (with their ends) fit six times in 40. + lines = [f"line{n}" for n in range(8)] + result = trunc_display_content('\n'.join(lines)) + assert result == { + 'content': '\n'.join(lines[:6]), + 'truncated': True, + } + + +def test_cut_lines_count_as_cut(small_limits: None) -> None: + # Lines cut, then the whole: only lines kept are listed. + content = '\n'.join(['y' * 20] * 10) + result = trunc_display_content(content) + assert result['truncated'] + assert result['content'] == '\n'.join(['y' * 8] * 4) + assert result['lines_truncated'] == [1, 2, 3, 4] + assert len(result['content']) <= 40 + + +def test_one_line_larger_than_everything( + monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(shared, 'MAX_DISPLAY_CONTENT_SIZE', 10) + monkeypatch.setattr(shared, 'MAX_DISPLAY_LINE_SIZE', 100) + assert trunc_display_content('z' * 50 + '\nmore') == { + 'content': 'z' * 10, + 'truncated': True, + 'lines_truncated': [1], + } + + +def test_no_characters_are_lost_between_lines(small_limits: None) -> None: + # The old walk back dropped the character before a line end. + lines = ['abcdefg'] * 10 + content = trunc_display_content('\n'.join(lines))['content'] + assert all(line == 'abcdefg' for line in content.split('\n')) + + +def test_large_content_is_linear() -> None: + # Many short lines past the content limit, and a huge one, go quickly. + content = 'a\n' * 8_000_000 + 'b' * 20_000_000 + start = time.monotonic() + result = trunc_display_content(content) + assert time.monotonic() - start < 20 + assert result['truncated'] + assert 'lines_truncated' not in result + + +def test_labels_end_in_an_ellipsis() -> None: + assert trunc_display_label('PASS') == 'PASS' + fits = 'x' * MAX_DISPLAY_LABEL_SIZE + assert trunc_display_label(fits) == fits + label = trunc_display_label(fits + 'x') + assert len(label) == MAX_DISPLAY_LABEL_SIZE + assert label.endswith('\N{HORIZONTAL ELLIPSIS}') + + +def test_display_reads_only_what_fits(small_limits: None) -> None: + taken: list[str] = [] + + def pieces() -> Iterator[tuple[str, bool]]: + for n in range(1000): + taken.append(f"{n}") + yield f"line{n}\n", True + + with contextlib.closing(pieces()) as generator: + shown = display_content(generator) + assert shown['truncated'] + # Up to the first line that doesn't fit, and no more. + assert len(taken) == 7 + assert shown['content'] == '\n'.join(f"line{n}" for n in range(6)) + + +def test_display_cuts_lines_across_pieces(small_limits: None) -> None: + # A line in several pieces; then a text without a last line end, which + # the next line doesn't run into. + pieces = [ + ('abcde', True), ('fghij', False), ('k\n', False), + ('no end', True), + ('# next\n', True), + ] + assert display_content(pieces) == { + 'content': 'abcdefgh\nno end\n# next\n', + 'lines_truncated': [1], + } diff --git a/dev_tools/services/acknowledg-forge/tests/test_github.py b/dev_tools/services/acknowledg-forge/tests/test_github.py index 3c333825a7..be2b08f64a 100644 --- a/dev_tools/services/acknowledg-forge/tests/test_github.py +++ b/dev_tools/services/acknowledg-forge/tests/test_github.py @@ -235,3 +235,38 @@ def handler(request: httpx.Request) -> httpx.Response: assert posted == [{'body': 'hello'}] assert await commenter.find_comment(REPO, 1, 'MARK') == '2' assert await commenter.find_comment(REPO, 1, 'OTHER') is None + + +async def test_responses_are_read_only_so_far( + monkeypatch: pytest.MonkeyPatch) -> None: + from edgacknowledg.forge import ForgeError + from edgacknowledg.forge import http as forge_http + import gzip + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == '/small': + # Compressed, and paginated: decoded once, links kept. + return httpx.Response( + 200, + headers = { + 'content-encoding': 'gzip', + 'link': f'<{API}/small?page=2>; rel="next"', + }, + content = gzip.compress(json.dumps({'ok': True}).encode()) + ) + return httpx.Response(200, json = {'pad': 'x' * 1000}) + + client = forge_http.ForgeHttpClient( + base_url = API, + headers = {}, + transport = httpx.MockTransport(handler) + ) + try: + monkeypatch.setattr(forge_http, 'MAX_RESPONSE_BYTES', 100) + response = await client.request('GET', '/small') + assert response.json() == {'ok': True} + assert response.links['next']['url'] == f"{API}/small?page=2" + with pytest.raises(ForgeError, match = 'over 100 bytes'): + await client.get_json('/large') + finally: + await client.aclose() diff --git a/dev_tools/services/acknowledg-forge/tests/test_rematerialize.py b/dev_tools/services/acknowledg-forge/tests/test_rematerialize.py index 7123d85ca2..05dcea285b 100644 --- a/dev_tools/services/acknowledg-forge/tests/test_rematerialize.py +++ b/dev_tools/services/acknowledg-forge/tests/test_rematerialize.py @@ -12,8 +12,18 @@ import sys from pathlib import Path, PurePosixPath +from typing import Any import edgtest +import edgutil + +# (Not 'from ... import TestSourceHandler', which pytest would collect.) +import edgacknowledg.client.test as test_provider + +from edgacknowledg import RequestTags, message +from edgacknowledg.client.shared import SandboxContext + +from test_symlinks import FakeChannel DEV_TOOLS = Path(__file__).resolve().parents[3] REMATERIALIZE = DEV_TOOLS / 'bin' / 'edg-rematerialize-test-run' @@ -155,7 +165,16 @@ def static_diff(test: str) -> str: edgtest.runtest.RUN_TEST_OUTPUT_SUFFIX ) with edgtest.runtest.RunTestOutput(rt_path) as rt_output: - return ''.join(rt_output.combined_static_diff()) + text = ''.join(rt_output.combined_static_diff()) + # As pieces (for display), the same, header lines fitting in one. + for limit in (len('--- Expected Output\n'), 1000): + assert ''.join( + piece for piece, _ in rt_output.iter_static_diff_pieces(limit) + ) == text + assert ''.join( + piece for piece, _ in rt_output.iter_raw_output_pieces(limit) + ) == ''.join(rt_output.combined_raw_output()) + return text c_diff = static_diff('suite/c.sft.cpp') assert '# cpfe -DTEST_NUMBER=1 Test_name.c\n' in c_diff @@ -179,3 +198,149 @@ def static_diff(test: str) -> str: for config, changes_file in changes_files.items() for test in changes_file.read_text().split() ) + + +def _rematerialize_change(tmp_path: Path, + change: dict[str, str]) -> tuple[ + subprocess.CompletedProcess[str], Path]: + '''Run the tool on the diff ``change`` makes to a small test home; return + its result and the run's directory. + ''' + repo = tmp_path / 'repo' + repo.mkdir() + git(repo, 'init', '--quiet', '--initial-branch=main') + write_files(repo, { + 'tests/.edgy/config.json': '{}\n', + 'tests/expectations/edg_x86_64/suite.log': '', + 'tests/expectations/edg_x86_64_cp/suite.log': '', + 'tests/tests/suite/a.sft.cpp': '//type:cp\nint a;\n', + 'tests/tests/suite/.a.rto/default.1.1.txt': ( + 'cpfe -DTEST_NUMBER=1 Test_name.c\nold output\n' + ), + }) + git(repo, 'add', '-A') + git(repo, 'commit', '--quiet', '-m', 'base') + write_files(repo, change) + git(repo, 'add', '-A') + git(repo, 'commit', '--quiet', '-m', 'change') + diff_file = tmp_path / 'pr.diff' + diff_file.write_text(git( + repo, 'diff', '--no-renames', '--no-color', 'HEAD~1', 'HEAD', '--', 'tests' + )) + git(repo, 'checkout', '--quiet', 'HEAD~1') + + runs_dir = tmp_path / 'runs' + timestamp = '2026.01.02-03.04.05' + env = dict(os.environ) + env['EDG_TEST_HOME'] = str(repo / 'tests') + env['PYTHONPATH'] = str(DEV_TOOLS / 'pylibs') + env.pop('RUN_TEST_DEFAULT_CONFIG', None) + result = subprocess.run( + [sys.executable, str(REMATERIALIZE), + '--runs-dir', str(runs_dir), + '--run-timestamp', timestamp, + str(diff_file)], + cwd = repo, + env = env, + capture_output = True, + text = True + ) + return result, runs_dir / timestamp + + +def test_rematerialize_passes_huge_lines_through(tmp_path: Path) -> None: + # Far longer than a line is ever held whole; written (and patched) in + # full. + huge = 'h' * (5 * edgutil.MAX_LINE_CHARS) + result, run_dir = _rematerialize_change(tmp_path, { + 'tests/tests/suite/.a.rto/default.1.1.txt': ( + f"cpfe -DTEST_NUMBER=1 Test_name.c\n{huge}\n" + ), + }) + assert result.returncode == 0, result.stdout + result.stderr + rt_path = (run_dir / 'edg_x86_64' / 'suite/a.sft.cpp').with_suffix( + edgtest.runtest.RUN_TEST_OUTPUT_SUFFIX + ) + with edgtest.runtest.RunTestOutput(rt_path) as rt_output: + static_diff = ''.join(rt_output.combined_static_diff()) + assert f"+{huge}\n" in static_diff + + +def test_rematerialize_refuses_huge_expectation_lines(tmp_path: Path) -> None: + result, _ = _rematerialize_change(tmp_path, { + 'tests/expectations/edg_x86_64/suite.log': ( + 'a.sft.cpp:cp:-DTEST_NUMBER=1:FAIL:' + + 'r' * edgutil.MAX_LINE_CHARS + '\n' + ), + }) + assert result.returncode == 1 + assert ( + 'tests/expectations/edg_x86_64/suite.log: a changed line is longer than' + ) in result.stderr + + +async def test_curr_diff_is_shown_and_compared(tmp_path: Path) -> None: + result, run_dir = _rematerialize_change(tmp_path, { + 'tests/tests/suite/.a.rto/default.1.1.txt': ( + 'cpfe -DTEST_NUMBER=1 Test_name.c\nnew output\n' + ), + # Brings the other configuration into the run (which inherits the + # recording's change). + 'tests/expectations/edg_x86_64_cp/suite.log': ( + 'a.sft.cpp:cp:-DTEST_NUMBER=1:FAIL:\n' + ), + }) + assert result.returncode == 0, result.stdout + result.stderr + context = SandboxContext(tmp_path) + handler = test_provider.TestSourceHandler( + 'Test', + 'test', + context, + context.subpath(tmp_path / 'repo' / 'tests'), + context.subpath(run_dir) + ) + + async def show_curr_diff() -> dict[str, Any]: + channel = FakeChannel(message( + RequestTags.TEST_SHOW_CURR_DIFF, + value = { + 'id': '(edg_x86_64) suite/a.sft.cpp', + 'configs': ['edg_x86_64', 'edg_x86_64_cp'], + 'reviewer_id': 'r', + } + )) + await handler.handle_request(channel) # type: ignore[arg-type] + [reply] = channel.sent + response: dict[str, Any] = reply['v']['client_response'] + return response + + # Observed against the run's (the pull request's) recordings, which it + # was made from: no difference. + response = await show_curr_diff() + assert response['content'] == '' + assert 'divergent_configs' not in response + + # Recordings changed since: the difference is shown, the same for both + # configurations (the other uses the default recording). + recordings = run_dir / 'patched-tests' / 'suite' / '.a.rto' + (recordings / 'default.1.1.txt').write_text( + 'cpfe -DTEST_NUMBER=1 Test_name.c\nnewer output\n' + ) + response = await show_curr_diff() + assert response['content'] == ( + '--- Expected Output\n' + '+++ Observed Output\n' + '# cpfe -DTEST_NUMBER=1 Test_name.c\n' + '@@ -1 +1 @@\n' + '-newer output\n' + '+new output\n' + ) + assert 'divergent_configs' not in response + + # A recording of its own makes the other configuration's differ. + (recordings / 'edg_x86_64_cp.1.1.txt').write_text( + 'cpfe -DTEST_NUMBER=1 Test_name.c\nnew output\n' + ) + response = await show_curr_diff() + assert '-newer output\n+new output' in response['content'] + assert response['divergent_configs'] == ['edg_x86_64_cp'] diff --git a/dev_tools/services/acknowledg-forge/tests/test_safety.py b/dev_tools/services/acknowledg-forge/tests/test_safety.py index 822538d76a..9ade8cc0ca 100644 --- a/dev_tools/services/acknowledg-forge/tests/test_safety.py +++ b/dev_tools/services/acknowledg-forge/tests/test_safety.py @@ -17,12 +17,18 @@ import pytest +import edgutil + +from edgacknowledg.forge import 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 ( UnsafeArchiveError, WorkspaceError, + Workspaces, + _OutputTooLarge, _central_directory_size, _communicate_bounded, changes_patterns, @@ -32,6 +38,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 +235,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(io.StringIO(diff)) 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 @@ -243,6 +252,63 @@ def test_test_patterns_are_linear_in_the_path() -> None: ] +def test_line_pieces_are_bounded_and_lose_nothing() -> None: + text = ( + 'diff --git a/tests/tests/s/a.sft.cpp b/tests/tests/s/a.sft.cpp\n' + '+' + 'x' * 1000 + '\n\n+last' + ) + pieces = list(edgutil.iter_line_pieces(io.StringIO(text), 16)) + assert ''.join(piece for piece, _ in pieces) == text + assert all(len(piece) <= 16 for piece, _ in pieces) + # Only a line's first piece starts it. + assert [piece for piece, at_line_start in pieces if at_line_start] == [ + 'diff --git a/tes', '+xxxxxxxxxxxxxxx', '\n', '+last' + ] + + +def test_limited_lines_refuse_overlong_lines() -> None: + # Up to the limit (its end not counted), lines come whole. + text = '0123456789\nshort\n' + 'y' * 11 + '\nnever read\n' + lines = edgutil.iter_limited_lines(io.StringIO(text), 10) + assert next(lines) == '0123456789\n' + assert next(lines) == 'short\n' + with pytest.raises(edgutil.LineTooLongError, + match = 'line 3 is longer than 10 characters'): + next(lines) + # A last line without an end is checked too. + def limited(text: str) -> list[str]: + return list(edgutil.iter_limited_lines(io.StringIO(text), 10)) + + assert limited('a\n' + 'b' * 10) == ['a\n', 'b' * 10] + with pytest.raises(edgutil.LineTooLongError): + limited('a\n' + 'b' * 11) + + +def test_changes_patterns_pass_over_huge_lines_in_pieces() -> None: + # A line far over the limit, in an expectations file, names no test (and + # is never held whole); the lines after it still count. + diff = ( + 'diff --git a/tests/expectations/x/s.log b/tests/expectations/x/s.log\n' + '+++ b/tests/expectations/x/s.log\n' + '+big.sft.cpp:' + 'q' * (4 * edgutil.MAX_LINE_CHARS) + '\n' + '+ok.sft.cpp:cp:-DTEST_NUMBER=1:PASS:\n' + ) + handle = io.StringIO(diff) + real_readline = handle.readline + sizes: list[int] = [] + + def readline(size: int = -1) -> str: + piece = real_readline(size) + sizes.append(len(piece)) + return piece + + handle.readline = readline # type: ignore[method-assign] + patterns = changes_patterns(handle) + assert '/tests/tests/s/ok.sft.cpp' in patterns + assert not any('big' in pattern for pattern in patterns) + assert max(sizes) <= edgutil.MAX_LINE_CHARS + 1 + + def test_changes_patterns_skip_suites_no_suite_may_be_named() -> None: # A suite is in the patterns of every test its expectation file names. def expectations(suite: str) -> str: @@ -254,10 +320,10 @@ def expectations(suite: str) -> str: '+a.sft.cpp: PASS', ]) - patterns = changes_patterns(expectations('cwg')) + patterns = changes_patterns(io.StringIO(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)) == [ + assert changes_patterns(io.StringIO(expectations(suite))) == [ '/tests/.edgy/', '/tests/expectations/', ] @@ -276,34 +342,112 @@ 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(io.StringIO(diff), max_patterns = cap) # Within a higher cap. - changes_patterns(diff, max_patterns = 3 * cap) + changes_patterns(io.StringIO(diff), 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_git_stdin_is_written_and_stderr_bounded() -> None: + sunk: list[bytes] = [] + # It reads its input (larger than a pipe holds) while writing more + # stderr than is kept. + process = await asyncio.create_subprocess_exec( + 'sh', '-c', 'cat; head -c 1000000 /dev/zero | tr "\\0" e >&2', + stdin = asyncio.subprocess.PIPE, + stdout = asyncio.subprocess.PIPE, + stderr = asyncio.subprocess.PIPE + ) + data = b'pattern\n' * 100_000 + stderr = await _communicate_bounded( + process, + len(data), + sunk.append, + data + ) + assert process.returncode == 0 + assert b''.join(sunk) == data + assert stderr == b'e' * workspace._MAX_GIT_STDERR_BYTES + + +async def test_git_results_are_bounded(tmp_path: Path, + forge: FakeForge, + monkeypatch: pytest.MonkeyPatch + ) -> None: + workspaces = Workspaces(tmp_path / 'home', forge) + assert (await workspaces.git('version')).stdout.startswith('git version') + monkeypatch.setattr(workspace, '_MAX_GIT_OUTPUT_BYTES', 4) + with pytest.raises(WorkspaceError, match = 'wrote over 4 bytes'): + await workspaces.git('version') + + +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. + with open(diff_file, encoding = 'utf-8') as handle: + assert '/tests/tests/s/1999.sft.cpp' in changes_patterns(handle) + + # 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 +456,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) @@ -348,6 +494,16 @@ def test_ci_reads_are_capped(tmp_path: Path, _read_ci_metadata(metadata) +def test_ci_test_ids_skip_overlong_lines(tmp_path: Path) -> None: + elist = tmp_path / 'a.elist' + elist.write_text( + 's/a.sft.cpp\n' + + 's/' + 'x' * processor.MAX_TEST_ID_CHARS + '\n' + + 's/b.sft.cpp\n' + ) + assert _changes_test_ids([elist]) == ['s/a.sft.cpp', 's/b.sft.cpp'] + + def test_failures_patterns_ignore_escaping_ids() -> None: patterns = failures_patterns(['s/a.sft.cpp', '../../etc/passwd']) assert '/tests/tests/s/a.sft.cpp' in patterns 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/services/acknowledg-forge/tests/test_symlinks.py b/dev_tools/services/acknowledg-forge/tests/test_symlinks.py index efb70626b4..baea625424 100644 --- a/dev_tools/services/acknowledg-forge/tests/test_symlinks.py +++ b/dev_tools/services/acknowledg-forge/tests/test_symlinks.py @@ -557,7 +557,6 @@ async def test_show_source_reads_only_what_is_shown( monkeypatch: pytest.MonkeyPatch) -> None: home, root = _test_tree(tmp_path, secret) (home / 'tests' / 'suite' / 'big.sft.cpp').write_text('int x;\n' * 1000) - monkeypatch.setattr(test_provider, 'MAX_DISPLAY_CONTENT_SIZE', 64) monkeypatch.setattr(shared, 'MAX_DISPLAY_CONTENT_SIZE', 64) reads: list[int] = [] real_open = shared.ConfinedPath.open @@ -566,12 +565,17 @@ def open_recording_reads(self: shared.ConfinedPath, *args: Any, **kwargs: Any) -> Any: handle = real_open(self, *args, **kwargs) - real_read = handle.read + real_readline = handle.readline + + def readline(size: int = -1) -> Any: + line = real_readline(size) + reads.append(len(line)) + return line def read(size: int = -1) -> Any: - reads.append(size) - return real_read(size) + raise AssertionError('read whole') + handle.readline = readline handle.read = read return handle @@ -594,7 +598,8 @@ def read(size: int = -1) -> Any: response = reply['v']['client_response'] assert response['truncated'] and len(response['content']) <= 64 assert response['content'].startswith('int x;\n') - assert reads == [65] + # A line at a time, and no more than one past what fits. + assert reads == [7] * 10 async def test_sandbox_context_commands(tmp_path: Path) -> None: diff --git a/dev_tools/services/acknowledg/PROTOCOL.md b/dev_tools/services/acknowledg/PROTOCOL.md index 07188c5904..a9c700388e 100644 --- a/dev_tools/services/acknowledg/PROTOCOL.md +++ b/dev_tools/services/acknowledg/PROTOCOL.md @@ -3,7 +3,7 @@ This document describes the WebSocket protocol used by the AcknowlEDG server, the Vue client, and the `edgy-review` / `edg-bench-review` / `edg-acknowledg-cli` providers. -The current protocol revision is **19**. +The current protocol revision is **20**. All frames are JSON text. Implementations live in `edgacknowledg` (`dev_tools/pylibs/edgacknowledg`) and must stay in sync with the JavaScript @@ -74,15 +74,15 @@ work can flow immediately. Providers may optionally send identity updates at any time (on dedicated sockets or nested in `batch.event`): ```json -{ "t": "meta.section.name", "r": 19, "v": "" } +{ "t": "meta.section.name", "r": 20, "v": "" } ``` ```json -{ "t": "meta.review.name", "r": 19, "v": [ ] } +{ "t": "meta.review.name", "r": 20, "v": [ ] } ``` ```json -{ "t": "meta.review.timestamp", "r": 19, "v": "" } +{ "t": "meta.review.timestamp", "r": 20, "v": "" } ``` - `meta.section.name` sets the display name for the section tab (shown in @@ -329,7 +329,7 @@ content that isn't the provider user's own, one reached through a symlink), the provider doesn't answer the request. It reports the reviewer instead: ```json -{ "t": "illegal-access", "r": 19, "v": { "reviewer_id": "" } } +{ "t": "illegal-access", "r": 20, "v": { "reviewer_id": "" } } ``` The server then closes that reviewer's socket with code **3001**, without @@ -345,7 +345,7 @@ access), it doesn't answer the request either. It tells the reviewer it failed, through the server, and keeps serving the section: ```json -{ "t": "invalid", "r": 19, "v": { "reviewer_id": "" } } +{ "t": "invalid", "r": 20, "v": { "reviewer_id": "" } } ``` The server sends `invalid` to that reviewer alone, with `v` `null` and `s` @@ -362,6 +362,19 @@ missing) and may set `truncated: true` when the body exceeds 8 MiB (`MAX_DISPLAY_CONTENT_SIZE`). Protocol messages themselves are capped at 10 MiB (`MAX_MESSAGE_SIZE`). +Each line of `content` is at most 16 Ki characters +(`MAX_DISPLAY_LINE_SIZE`). A provider cuts a longer line to that length and +lists its 1-based line number in `lines_truncated` (sorted ascending, and +omitted when no line was cut). Nothing in `content` itself marks a cut. The +reviewer UI appends `…` (U+2026) to each listed line and shows a "Some Lines +Truncated" notice. Line cuts happen before the 8 MiB cap, so `truncated` and +`lines_truncated` may both be set; `lines_truncated` lists only lines +present in `content`. + +Single-line labels (the `qualifier`, `variant`, and `status` of +`test.src.list-statuses` lines, and its `remark`) are cut to 1 Ki characters +(`MAX_DISPLAY_LABEL_SIZE`), ending in `…`. + **`test.src.list-statuses`** ```json @@ -392,7 +405,12 @@ missing) and may set `truncated: true` when the body exceeds 8 MiB **`test.src.show-diff`**, **`test.src.show-output`** ```json -{ "id": "", "content": "", "truncated": false } +{ + "id": "", + "content": "", + "truncated": false, + "lines_truncated": [1] +} ``` **`test.src.show-diff-curr`** — same as above, plus optional @@ -412,7 +430,8 @@ match the representative. "id": "", "assoc_file": "", "content": "", - "truncated": false + "truncated": false, + "lines_truncated": [1] } ``` @@ -434,7 +453,12 @@ match the representative. **`bench.src.show-annotate`** ```json -{ "id": "", "content": "", "truncated": false } +{ + "id": "", + "content": "", + "truncated": false, + "lines_truncated": [1] +} ``` ### Editor `v` as seen by the reviewer diff --git a/dev_tools/services/acknowledg/client/src/BenchReviewView.vue b/dev_tools/services/acknowledg/client/src/BenchReviewView.vue index 63484d1cd0..e469749601 100644 --- a/dev_tools/services/acknowledg/client/src/BenchReviewView.vue +++ b/dev_tools/services/acknowledg/client/src/BenchReviewView.vue @@ -22,6 +22,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception } from './bench-metrics.js' import { formatBenchAnchor, parseBenchAnchor } from './review-anchor.js' import { protocolMessage } from './sockets.js' + import { displayLines } from './display-content.js' import AppButton from './AppButton.vue' import AppSelect from './AppSelect.vue' import BenchStatus from './BenchStatus.vue' @@ -47,8 +48,14 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception ] const selectedBenchmarkId = ref(null) const displayTruncated = ref(false) + // 1-based numbers of the displayed lines the provider cut. + const displayedLinesTruncated = ref([]) const displayedOutput = ref(null) + const displayedOutputLines = computed(() => { + return displayLines(displayedOutput.value, displayedLinesTruncated.value) + }) + provide('activeBenchmarkId', selectedBenchmarkId) provide('benchSortMode', sortMode) @@ -177,7 +184,8 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception const sorted = benchmarks.value.slice() sorted.sort((left, right) => { const leftChange = computePercentChange(left.baseline_ops, left.run_ops) - const rightChange = computePercentChange(right.baseline_ops, right.run_ops) + const rightChange = computePercentChange(right.baseline_ops, + right.run_ops) if (sortMode.value === 'greatest-speedup') { return rightChange - leftChange } @@ -212,12 +220,14 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception } else if (message.t === 'bench.src.show-annotate') { if (message.v.id === selectedBenchmarkId.value) { displayTruncated.value = false + displayedLinesTruncated.value = [] if (message.v.content === null) { displayedOutput.value = 'Annotation unavailable!' } else { if (message.v.truncated) { displayTruncated.value = true } + displayedLinesTruncated.value = message.v.lines_truncated ?? [] displayedOutput.value = message.v.content } } @@ -353,7 +363,8 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
Average Change (OPs)
{{ formatSignedOps(Math.round(mean(sampleOpDeltas))) }} - σ {{ Math.round(populationStdev(sampleOpDeltas)).toLocaleString() }} + σ {{ Math.round(populationStdev(sampleOpDeltas)) + .toLocaleString() }}
@@ -404,15 +415,21 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception class="truncation-warning"> Content Too Long: Display Truncated
+
+ Some Lines Truncated +
- 
{{ displayedOutput }}
+ 
{{ displayedOutputLines.join('\n') }}
-
+
Select a benchmark to continue...
diff --git a/dev_tools/services/acknowledg/client/src/TestReviewView.vue b/dev_tools/services/acknowledg/client/src/TestReviewView.vue index ccc35b99a9..82d6eaea56 100644 --- a/dev_tools/services/acknowledg/client/src/TestReviewView.vue +++ b/dev_tools/services/acknowledg/client/src/TestReviewView.vue @@ -19,6 +19,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception import { protocolMessage } from './sockets.js' import { computeGradientStyleForConfigs } from './config-color.js' import { formatTestAnchor, parseTestAnchor } from './review-anchor.js' + import { displayLines } from './display-content.js' import DiffLine from './DiffLine.vue' import LoadingSpinner from './LoadingSpinner.vue' @@ -44,11 +45,17 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception const displayedTestAssocFiles = ref(null) const displayedTestAssocFileName = ref(null) const displayTruncated = ref(false) + // 1-based numbers of the displayed lines the provider cut. + const displayedLinesTruncated = ref([]) const currDiffDivergenceNotice = ref(null) const displayedOutput = ref("") const outputViewModeTarget = ref("test.src.show-diff") const isMultiConfig = ref(false) + const displayedOutputLines = computed(() => { + return displayLines(displayedOutput.value, displayedLinesTruncated.value) + }) + const displayedTestConfig = computed(() => { return displayedGroupConfigs.value[0] ?? "" }) @@ -279,10 +286,12 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception isMultiConfig.value = message.v.configs.length > 1 testStatuses.value = message.v.statuses applyHashSelection() - } else if (message.t === "test.src.show-diff" || message.t === "test.src.show-diff-curr" || + } else if (message.t === "test.src.show-diff" || + message.t === "test.src.show-diff-curr" || message.t === "test.src.show-output") { if (message.v.id === configuredIdForDisplayedTest()) { displayTruncated.value = false + displayedLinesTruncated.value = [] currDiffDivergenceNotice.value = null if (message.v.content === null) { displayedOutput.value = "Missing .rt.tar file!" @@ -290,6 +299,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception if (message.v.truncated) { displayTruncated.value = true } + displayedLinesTruncated.value = message.v.lines_truncated ?? [] displayedOutput.value = message.v.content } if (message.t === "test.src.show-diff-curr" && @@ -307,6 +317,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception } else if (message.t === "test.src.show-src") { if (message.v.id === displayedTestName.value) { displayTruncated.value = false + displayedLinesTruncated.value = [] currDiffDivergenceNotice.value = null if (message.v.content === null) { displayedOutput.value = "File not found!" @@ -314,6 +325,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception if (message.v.truncated) { displayTruncated.value = true } + displayedLinesTruncated.value = message.v.lines_truncated ?? [] displayedOutput.value = message.v.content } } @@ -462,7 +474,8 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
+
@@ -487,6 +500,11 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception class="content-notice"> Content Too Long: Display Truncated
+
+ Some Lines Truncated +
{{ currDiffDivergenceNotice }} @@ -499,21 +517,23 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception style="overflow: visible; display: grid;">
-
+
Select a test status difference to continue...
-
+
{ + if (content === null || content === undefined) { + return [] + } + const lines = content.split('\n') + for (const lineNumber of linesTruncated) { + lines[lineNumber - 1] += TRUNCATION_MARK + } + return lines +} diff --git a/dev_tools/services/acknowledg/client/src/sockets.js b/dev_tools/services/acknowledg/client/src/sockets.js index cd57a09e9a..3a5b2f98ac 100644 --- a/dev_tools/services/acknowledg/client/src/sockets.js +++ b/dev_tools/services/acknowledg/client/src/sockets.js @@ -5,7 +5,7 @@ See https://edgcpp.org/LICENSE.txt for license information. SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception */ -const PROTOCOL_REVISION = 19 +const PROTOCOL_REVISION = 20 const getSocketHost = () => { if (process.env.NODE_ENV === "development") { @@ -26,7 +26,9 @@ export const acquireSocketForReview = (reviewId) => { ) } -export const protocolMessage = (tag, value = undefined, section = undefined) => { +export const protocolMessage = (tag, + value = undefined, + section = undefined) => { const message = { t: tag, r: PROTOCOL_REVISION } if (value !== undefined) { message.v = value 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()