Repository navigation
test(testkit): add cleanup hooks that run after pass, failure, or panic - #5219
Conversation
Tests can register cleanup with cx.on_cleanup(name, hook). Hooks run last registered first after the body, each with CLEANUP_TIMEOUT and caught panics. A failing, panicking, or timed-out hook is reported, the rest still run, and a test whose body passed fails. Sandbox tracking now registers a delete-and-wait hook keyed by sandbox name, so tracking then adopting registers it once and Sandbox::delete removes it. Leftover sandboxes are deleted one at a time rather than concurrently. The Keycloak provider-refresh test registers its provider and profile deletes right after creating them, instead of deleting them inline after the body, so cleanup also runs on panic and a failed delete fails the test. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Check for and add a keyed hook under one guard, so concurrent tracking of the same sandbox registers one delete. Rename the double-tracking test and count the deletes it reports rather than relying on the error text. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
|
Label |
… them Both deletes succeed when there is nothing to delete, so registering them first also cleans up a create the gateway applied but the CLI reported as failed. Show the same order in the on_cleanup docs. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…anup-hooks/matthewgrossman Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
|
/ok-to-test a6a2677 |
elezar
left a comment
elezar
left a comment
There was a problem hiding this comment.
LGTM. The hook list replaces the sandbox name registry cleanly, per-hook panic and timeout isolation is covered by the unit tests, and branch E2E (conformance and the Keycloak suite) is green.
Non-blocking: I'll send a follow-up PR once this lands that types the hook key (Hook.key: Option<String> becomes a key enum) and drops the Option key parameter from Hook::new in favor of a keyed(key) builder. No change needed here.
|
Follow-up is up: #5240. |
Summary
Add testkit cleanup hooks:
cx.on_cleanup(name, async move || ...)runs after the test body whether it passes, fails, or panics. Sandbox cleanup and the Keycloak suite's provider and profile deletes now use them. This is the testkit change proposed in the #5137 review; #5137 rebases onto it.Related Issue
Closes #5218
Changes
tests/testkit/testkit/src/cleanup.rs. Hooks run last registered first. Each is limited toCLEANUP_TIMEOUT(now public) and its panics are caught. A hook that fails, panics, or times out is reported, the rest still run, and a test whose body passed fails.Context::on_cleanupregisters a hook.Context::cli_handlereturns the test's CLI, for hooks that can't borrowcx.Sandbox::deleteremoves the hook. This replaces the sandbox name registry.runkeeps gateway setup. The body, cleanup, and outcome logic move torun_in, so unit tests can run them against an unconnectedContext.tests/testkit/README.mdand the Keycloak fixture docs describe the hooks.Leftover sandboxes are now deleted one at a time instead of concurrently, as proposed. Each testkit test leaves at most one sandbox today. Each hook gets its own
CLEANUP_TIMEOUT(2 minutes), the same as the runner's limit per test, so a hung hook can get the test killed during cleanup. Concurrent deletes had the same problem with a single hung delete. Fitting cleanup within the runner's limit is a separate follow-up.Testing
cargo nextest runfortests/testkit(12 tests), clippy and fmt forkeycloak-provider-refresh, clippy-D warningsfortests/testkit,tests/suites/conformance,tests/suites/features, andtests/support,mise run rust:format:check, rustdoc, and the lockfile alignment check passrun_inwith a passing, failing, or panicking body. In the panicking case, the test fails with the body's panic and the hook's marker is set.Checklist