Copilot commented on code in PR #13035:
URL: https://github.com/apache/trafficserver/pull/13035#discussion_r3012454186
##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -346,4 +367,33 @@ def ATSReplayTest(obj, replay_file: str):
return tr
+def _load_ats_replay(obj, replay_file: str):
+ replay_config, autest_config = _load_replay_config(obj, replay_file)
+
+ summary = _get_replay_summary(autest_config)
+ if summary and not obj.Summary:
+ obj.Summary = summary
+
+ if _is_list_action(obj):
+ return None
+
+ return _build_ats_replay_test(obj, replay_file, autest_config)
+
+
+def _load_ats_replay_test_file(obj):
+ return _load_ats_replay(obj, obj.TestFile)
+
+
+def ATSReplayTest(obj, replay_file: str):
+ '''Create a TestRun that configures ATS and runs HTTP traffic using the
replay file.
+
+ :param obj: The Test object to add the test run to.
+ :param replay_file: Replay file specifying the test configuration and test
traffic.
+ :returns: The TestRun object.
+ '''
+
+ return _load_ats_replay(obj, replay_file)
+
+
ExtendTest(ATSReplayTest, name="ATSReplayTest")
+RegisterTestFormat(_load_ats_replay_test_file, "ATSReplayYAMLTest",
ext=ATS_REPLAY_TEST_EXTENSIONS)
Review Comment:
This registers a new AuTest test format via `RegisterTestFormat`, which is
called unconditionally at import time. The repository’s AuTest minimum version
check in `init.cli.ext` still pins to 1.10.4; if that version doesn’t provide
`RegisterTestFormat` (as implied by the upstream dependency), test
initialization will fail before any tests run. Update the minimum required
AuTest version (or guard the registration when the API isn’t available) so the
version gate matches the new dependency.
```suggestion
if 'RegisterTestFormat' in globals():
RegisterTestFormat(_load_ats_replay_test_file, "ATSReplayYAMLTest",
ext=ATS_REPLAY_TEST_EXTENSIONS)
```
##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -205,24 +207,43 @@ def _requires_persistent_ats(ats_config: dict) -> bool:
return bool(ats_config.get('metric_checks'))
-def ATSReplayTest(obj, replay_file: str):
- '''Create a TestRun that configures ATS and runs HTTP traffic using the
replay file.
+def _is_list_action(obj) -> bool:
+ return obj.Variables.Autest.Action == 'list'
+
+
Review Comment:
List-mode detection here uses `obj.Variables.Autest.Action == 'list'`, but
the CLI/setup changes in this PR key off `Arguments.subcommand == 'list'`. If
`Variables.Autest.Action` isn’t set by AuTest, `autest list` may either crash
(AttributeError) or proceed to build full ATS/Proxy Verifier processes,
defeating the goal of lightweight discovery. Consider using a single source of
truth (e.g., propagate subcommand into Variables in setup, or use a safe
`getattr`/`hasattr` check here) so list-mode reliably skips process
construction.
```suggestion
action = getattr(getattr(getattr(obj, 'Variables', None), 'Autest',
None), 'Action', None)
if action is not None:
return action == 'list'
subcommand = getattr(getattr(obj, 'Arguments', None), 'subcommand', None)
return subcommand == 'list'
```
##########
tests/README.md:
##########
@@ -34,8 +34,9 @@ To run autest again, or to run individual tests, the cmake
build generates a hel
To run a single test, you can use the `--filter` flag to name
which test to run. The tests are in files whose names are the test name
-, and are suffixed with `.test.py`. Thus, the `something_descriptive`
-test will be specified in a file named `something_descriptive.test.py`.
+, and are typically suffixed with `.test.py` or `.test.yaml`. Thus, the
+`something_descriptive` test will be specified in a file named
+`something_descriptive.test.py` or `something_descriptive.test.yaml`.
Review Comment:
The code supports both `.test.yaml` and `.test.yml` (see
`ATS_REPLAY_TEST_EXTENSIONS`), but this doc only mentions `.test.yaml`.
Consider mentioning `.test.yml` too so users know both forms are discoverable.
```suggestion
, and are typically suffixed with `.test.py`, `.test.yaml`, or `.test.yml`.
Thus, the
`something_descriptive` test will be specified in a file named
`something_descriptive.test.py`, `something_descriptive.test.yaml`, or
`something_descriptive.test.yml`.
```
--
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]
