Copilot commented on code in PR #13540:
URL: https://github.com/apache/trafficserver/pull/13540#discussion_r3908055042
##########
tests/autest-parallel.py.in:
##########
@@ -433,7 +487,7 @@ def run_single_test(test: str, script_dir: Path, sandbox:
Path, ats_bin: str, bu
cmd = [
'uv', 'run', 'autest', 'run', '--directory', '${CMAKE_GOLD_DIR}',
'--ats-bin', ats_bin, '--proxy-verifier-bin',
'${PROXY_VERIFIER_PATH}', '--build-root', build_root, '--sandbox',
- str(sandbox / test), '--filters', test
+ str(sandbox / test), '--filters', f'/{test}'
Review Comment:
Using f'/{test}' as an autest filter can still match multiple tests when one
test name is a prefix of another (e.g., rate_limit matches rate_limit_sni).
This risks reintroducing duplicate/incorrect test selection in parallel runs.
This issue also appears on line 592 of the same file.
##########
tests/autest-parallel.py.in:
##########
@@ -39,12 +39,26 @@ from concurrent.futures import ProcessPoolExecutor,
as_completed
from dataclasses import dataclass, field
from datetime import datetime
from pathlib import Path
-from typing import Dict, List, Optional, Tuple
+from queue import Empty, Queue
+from threading import Thread
+from typing import Dict, List, Optional, TextIO, Tuple
# Default serial tests file location
DEFAULT_SERIAL_TESTS_FILE = Path("${CMAKE_CURRENT_SOURCE_DIR}") /
"serial_tests.txt"
# Default estimate for unknown tests (seconds)
DEFAULT_TEST_TIME = 15.0
+# Maximum time between completed tests in a worker batch.
+PER_TEST_TIMEOUT = 600.0
Review Comment:
PER_TEST_TIMEOUT introduces a hard 600s cap between test completions when
running with -v/--collect-timings, which can terminate legitimately slow tests
and changes behavior compared to the 1h worker timeout used in the non-verbose
path.
##########
tests/autest-parallel.py.in:
##########
@@ -227,6 +241,46 @@ def strip_ansi(text: str) -> str:
return ansi_escape.sub('', text)
+def parse_test_completion(line: str) -> Optional[Tuple[str, str]]:
+ """Extract a completed test name and status from an autest progress
line."""
+ clean = strip_ansi(line).strip()
+ marker = 'Running Test '
+ marker_pos = clean.rfind(marker)
+
+ if marker_pos < 0:
+ return None
+
+ completion = clean[marker_pos + len(marker):]
+ match = re.match(rf'([^:\s]+):.*\b({TERMINAL_STATUS_PATTERN})\s*$',
completion, re.IGNORECASE)
+ if not match:
+ return None
+
+ return match.group(1), match.group(2).upper()
Review Comment:
parse_test_completion() only recognizes progress lines starting with
"Running Test <name>:". Elsewhere in this same file, parse_autest_output()
expects autest to emit completion lines like "Test: <name>: Passed/Failed", so
timing collection can miss completions, never reset the timeout, and fail to
produce per-test timings.
This issue also appears on line 261 of the same file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
