[I] Uranium follow-ups from #13545 [trafficserver]

["bryancall (via GitHub)" <gi...@apache.org>]

bryancall opened a new issue, #13877:
URL: https://github.com/apache/trafficserver/issues/13877

   #13545 moved the end-to-end suite from AuTest to Uranium. It was merged with review items still open, so that master would stop drifting from the branch. This issue tracks those items. The first two matter most: the first gets harder to fix with every new test, and the second is already broken.
   
   ## Do these first
   
   - [ ] **Procedural tests don't check diags.log for errors.** AuTest's `MakeATSProcess` failed any test whose `diags.log` contained `ERROR:`, `FATAL:` or `Unrecognized configuration value`, unless the test set `disable_log_checks`. Uranium only does this for replay manifests (`tests/tools/uranium/replay.py:805-811`). A procedural test's `ATS.close()` checks only `FATAL:` (`tests/tools/uranium/services/ats.py:649`). So `disable_log_checks=` is accepted and ignored, even though `h2/origin_test_helpers.py:30` documents it as replacing the default checks. Fix: have `ATS.close()` apply the same three exclusions unless `disable_log_checks` is set, and add a framework self-test where a procedural test with `ERROR:` in diags goes red. Some tests will then need `disable_log_checks=True`. Also mention it in `.claude/skills/convert-autest-to-urtest/SKILL.md`.
   - [ ] **`contrib/docker/ubuntu/resolute/Dockerfile:87` no longer configures.** It still passes `-DENABLE_AUTEST=OFF`, which `CMakeLists.txt:167-168` now turns into a fatal error. Switch it to `ENABLE_URTEST`.
   
   ## Assertions lost in conversion
   
   - [ ] **`remap/test_virtualhost_remap.py:110-112`.** The refused reload runs without `-m` and expects exit 0, and `wait_for_status(..., "fail")` is a substring match that always succeeds. Changing `ctx.fail` to `ctx.complete` at `src/proxy/VirtualHost.cc:545` leaves the test green. Assert the task status from the JSON-RPC response, or restore `-m` with `expected_return_codes=(2,)`.
   - [ ] **`remap/replay/remap_https.test.yaml`** checks status codes only, where the AuTest checked eight runs against gold files. It also sends `Content-Length: 0` from every origin, so the plaintext chunked path that #13763 meant to keep is gone. Both `remap-https-200_2.gold` files are now unreferenced.
   - [ ] **`tls/test_tls_sni_groups.py:116-121,129-134`** pins `-tls1_2` / `-tls1_3` on the client, so it no longer proves the server enforces `valid_tls_versions_in`.
   - [ ] **`tls/test_tls_resumption_scope.py:221-224,261-262`** checks only the `traffic_ctl` exit code. The AuTests also required the `sni.yaml` and `ssl_multicert.yaml` reload tasks. The client-CA scope case counts only `^200 bbb.example.com$` in squid.log, where the AuTest allowed no `bbb.example.com` line at all.
   - [ ] **`pluginTest/slice/test_slice_stale_generation.py:258-261`.** The regex alternation isn't grouped, so only one branch requires `key mixed`.
   - [ ] **`pluginTest/rate_limit/rate_limit_sni_exclude.test.yaml`** can't tell an excluded SNI from a limited one on admission. Add an exclude for `Limiter on limited.example.com is excluded`. This gap came over from the AuTest.
   
   ## Framework
   
   - [ ] **The sandbox-name collision guard crashes under xdist.** `tests/tools/uranium/plugin.py:123-128` raises `UsageError` on every worker, so a `-n 2` run with two files that both define `test_same` ends in `INTERNALERROR` (`assert not crashitem`). Do the check on the controller, or skip it when `hasattr(config, "workerinput")`.
   - [ ] **`plugin.py:105` catches only `BlockingIOError`.** An `flock` that fails with `ENOLCK` on a user-supplied `--sandbox` escapes as `INTERNALERROR`. Catch `OSError` and raise a `UsageError` that names the directory.
   - [ ] **Sandbox names drop the file path** (`runtime.py:186-194`), so two files that both define `test_foo` share one directory across selective runs. Add the module stem to the name, or document it.
   - [ ] **`runtime.py:168-183`.** `item_sandbox` and `procedural_sandbox` are identical, and `item_sandbox` never reads `replay_path`. Reverting `parent=` in `replay.py` leaves the self-tests green, so add a self-test that does.
   - [ ] **`tests/tools/uranium/tests/test_replay.py:29`** covers `stop_after_client` only as `False` and `True`, never with the key absent.
   - [ ] **`jsonrpc/config_reload_helpers.py:81`.** `wait_for_status` matches summary text, so a root task in `created` can return early. The fixed `sleep(15)` checks in `test_config_reload_dedup.py`, `test_config_reload_reserve_subtask.py` and `test_config_reload_full_smoke.py` have the same weakness.
   - [ ] **`tools/uranium/tests/test_config.py:100-111`** no longer pins the test inventory. Add a check that no `*.test.py` remains under `tests/uranium_tests`.
   
   ## Docs and stale references
   
   - [ ] `doc/developer-guide/testing/config-reload-ext.en.rst` documents `Test.AddConfigReload()`, which no longer exists, and `doc/developer-guide/config-reload-framework.en.rst:1194-1208` names the old `config_reload_*.test.py` files.
   - [ ] `doc/developer-guide/api/functions/TSHttpHookAdd.en.rst:148-149` still names `test_hooks.test.py`.
   - [ ] `plugins/experimental/jax_fingerprint/README:154,161` still says AuTests.
   - [ ] `ci/rat-exclude.txt:79` names `tests/uranium_tests/autest-site/min_cfg`, which doesn't exist. The files are in `tests/tools/uranium/min_cfg`.
   - [ ] `tests/README.md:130-131` still says to write tests as a scenario class with `run()`.
   
   ## Check or clean up
   
   - [ ] `tls/replay/dual_cert_default_ec_rsa.replay.yaml:24` now lists `signed-san-ec.pem,signed-san.pem`, while master had RSA first. Confirm the reorder was intended, since the mixed layout is no longer tested.
   - [ ] `pluginTest/slice/replay/slice_body_headers.test.yaml:68-70` adds `Connection: close` to `/dup_te`, which master's replay didn't have. Confirm it isn't hiding a Uranium-only problem.
   - [ ] Orphaned files: `timeout/slow_server_uds.yaml` (the UDS variant of `inactive_client_timeout` no longer runs), `session_sharing/gold/200_two.gold`, `h2/gold/h2o-pool-reuse-metrics.gold`. Either wire them up or delete them.
   - [ ] Running as root, `test_ssl_multicert_partial_reload.py` fails because the generated 0600 keys are owned by root, and `test_runroot_verify` in `runroot/test_runroot.py` fails because root can read the 0400 file. `tests/urtest.sh` runs Podman as container root by default.
   


-- 
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: issues-unsubscribe@trafficserver.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Uranium follow-ups from #13545 [trafficserver]

["bneradt (via GitHub)" <gi...@apache.org>]

bneradt closed issue #13877: Uranium follow-ups from #13545
URL: https://github.com/apache/trafficserver/issues/13877


-- 
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: issues-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org