test(testkit): add cleanup hooks that run after pass, failure, or panic

tests/suites/features/keycloak-provider-refresh/tests/provider_refresh.rs

@@ -27,108 +27,107 @@ async fn revoked_refresh_grant_requires_user_reauthorization(cx: &mut Context) -
     let profile = write_profile(&profile_id, kc.keycloak())?;
     let profile_path = profile.path().to_string_lossy().into_owned();
 
-    let result = async {
-        kc.cli(["profile", "import", "--file", &profile_path])
-            .await?
-            .success()?;
-        kc.cli_with_env(
-            &[(CREDENTIAL_KEY, &grant.access_token)],
-            [
-                "provider",
-                "create",
-                "--name",
-                &provider,
-                "--type",
-                &profile_id,
-                "--credential",
-                CREDENTIAL_KEY,
-            ],
-        )
-        .await?
-        .success()?;
-        let client_id = format!("client_id={}", kc.keycloak().client_id());
-        kc.cli_with_env(
-            &[("KEYCLOAK_REFRESH_TOKEN", &grant.refresh_token)],
-            [
-                "provider",
-                "refresh",
-                "configure",
-                &provider,
-                "--credential-key",
-                CREDENTIAL_KEY,
-                "--strategy",
-                "oauth2-refresh-token",
-                "--material",
-                &client_id,
-                "--secret-material-env",
-                "refresh_token=KEYCLOAK_REFRESH_TOKEN",
-            ],
-        )
+    // Register each delete before its create, so cleanup also covers a create
+    // the gateway applied but the CLI reported as failed. Both deletes succeed
+    // when there is nothing to delete.
+    delete_on_cleanup(&kc, "profile", &profile_id);
+    kc.cli(["profile", "import", "--file", &profile_path])
         .await?
         .success()?;
+    delete_on_cleanup(&kc, "provider", &provider);
+    kc.cli_with_env(
+        &[(CREDENTIAL_KEY, &grant.access_token)],
+        [
+            "provider",
+            "create",
+            "--name",
+            &provider,
+            "--type",
+            &profile_id,
+            "--credential",
+            CREDENTIAL_KEY,
+        ],
+    )
+    .await?
+    .success()?;
+    let client_id = format!("client_id={}", kc.keycloak().client_id());
+    kc.cli_with_env(
+        &[("KEYCLOAK_REFRESH_TOKEN", &grant.refresh_token)],
+        [
+            "provider",
+            "refresh",
+            "configure",
+            &provider,
+            "--credential-key",
+            CREDENTIAL_KEY,
+            "--strategy",
+            "oauth2-refresh-token",
+            "--material",
+            &client_id,
+            "--secret-material-env",
+            "refresh_token=KEYCLOAK_REFRESH_TOKEN",
+        ],
+    )
+    .await?
+    .success()?;
 
-        let refresh = |verb: &'static str| {
-            [
-                "provider",
-                "refresh",
-                verb,
-                provider.as_str(),
-                "--credential-key",
-                CREDENTIAL_KEY,
-            ]
-        };
+    let refresh = |verb: &'static str| {
+        [
+            "provider",
+            "refresh",
+            verb,
+            provider.as_str(),
+            "--credential-key",
+            CREDENTIAL_KEY,
+        ]
+    };
 
-        kc.cli(refresh("rotate")).await?.success()?;
-        let status = kc.cli(refresh("status")).await?.success()?;
-        testkit::ensure!(
-            combined(&status).contains("refreshed"),
-            "a valid refresh did not reach the refreshed state:\n{}",
-            combined(&status)
-        );
+    kc.cli(refresh("rotate")).await?.success()?;
+    let status = kc.cli(refresh("status")).await?.success()?;
+    testkit::ensure!(
+        combined(&status).contains("refreshed"),
+        "a valid refresh did not reach the refreshed state:\n{}",
+        combined(&status)
+    );
 
-        kc.keycloak().revoke(&grant.refresh_token).await?;
-        let rotate = kc.cli(refresh("rotate")).await?.failure()?;
-        testkit::ensure!(
-            combined(&rotate).contains("invalid_grant"),
-            "a revoked refresh failed without invalid_grant:\n{}",
-            combined(&rotate)
-        );
+    kc.keycloak().revoke(&grant.refresh_token).await?;
+    let rotate = kc.cli(refresh("rotate")).await?.failure()?;
+    testkit::ensure!(
+        combined(&rotate).contains("invalid_grant"),
+        "a revoked refresh failed without invalid_grant:\n{}",
+        combined(&rotate)
+    );
 
-        let status = combined(&kc.cli(refresh("status")).await?.success()?);
-        for expected in [
-            "reauthorization_required",
-            "reauthorize",
-            "oauth_invalid_grant",
-        ] {
-            testkit::ensure!(
-                status.contains(expected),
-                "the revoked refresh status omits {expected}:\n{}",
-                status
-            );
-        }
-        // A parked refresh is scheduled at i64::MAX; it must not render as a date.
+    let status = combined(&kc.cli(refresh("status")).await?.success()?);
+    for expected in [
+        "reauthorization_required",
+        "reauthorize",
+        "oauth_invalid_grant",
+    ] {
         testkit::ensure!(
-            !status.contains("292278994"),
-            "the parked refresh rendered its scheduling sentinel as a date:\n{}",
+            status.contains(expected),
+            "the revoked refresh status omits {expected}:\n{}",
             status
         );
-        Ok(())
     }
-    .await;
+    // A parked refresh is scheduled at i64::MAX; it must not render as a date.
+    testkit::ensure!(
+        !status.contains("292278994"),
+        "the parked refresh rendered its scheduling sentinel as a date:\n{}",
+        status
+    );
+    Ok(())
+}
 
-    // testkit cleans up sandboxes, not providers or profiles, so remove them
-    // here whether or not the test passed. A panic or a killed test process
-    // skips this; the names are unique to the run, so leftovers don't collide.
-    // A failed cleanup doesn't fail the test, but it is reported.
-    for args in [
-        ["provider", "delete", provider.as_str()],
-        ["profile", "delete", profile_id.as_str()],
-    ] {
-        if let Err(error) = kc.cli(args).await.and_then(CliOutput::success) {
-            eprintln!("cleanup failed: {error}");
-        }
-    }
-    result
+/// Delete the `kind` named `name` when the test ends, since testkit only
+/// cleans up sandboxes by itself.
+fn delete_on_cleanup(cx: &Context, kind: &'static str, name: &str) {
+    let cli = cx.cli_handle();
+    let name = name.to_string();
+    cx.on_cleanup(format!("delete {kind} {name}"), async move || {
+        cli.run([kind, "delete", &name]).await?.success()?;
+        Ok(())
+    });
 }
 
 fn combined(output: &CliOutput) -> String {

tests/support/keycloak/src/lib.rs

@@ -34,7 +34,8 @@
 //! ```
 //!
 //! The fixture owns no Keycloak state, so it has nothing to clean up. Tests
-//! that create resources in Keycloak or `OpenShell` must remove them.
+//! that create resources in Keycloak or `OpenShell` must remove them, for
+//! example with [`Context::on_cleanup`].
 
 use std::fmt;
 use std::ops::{Deref, DerefMut};

tests/testkit/README.md

@@ -37,9 +37,27 @@ async fn long_running_exec_delete(cx: &mut Context) -> testkit::Result {
   refused, assert the failure in the body, for example with
   `cx.cli([...]).await?.failure()?` and `testkit::ensure!`.
 
-Cleanup runs after every test, whether it passes, fails, or panics. It only
-deletes sandboxes created through `cx.sandbox(...)`. Delete anything else a
-test creates, such as providers, in the test itself.
+Cleanup runs after every test, whether it passes, fails, or panics. It
+deletes the sandboxes created through `cx.sandbox(...)` or tracked with
+`cx.track(name)`, and runs the hooks registered with `cx.on_cleanup`. Register
+a hook for anything else a test creates, such as a provider. Register it before
+the create when the delete succeeds for a missing resource, so it also covers a
+create the gateway applied but the CLI reported as failed:
+
+```rust
+let cli = cx.cli_handle();
+let name = provider.clone();
+cx.on_cleanup(format!("delete provider {provider}"), async move || {
+    cli.run(["provider", "delete", &name]).await?.success()?;
+    Ok(())
+});
+cx.cli(["provider", "create", "--name", &provider /* ... */]).await?.success()?;
+```
+
+Hooks and sandbox deletes run one at a time, last registered first, each
+limited to `testkit::CLEANUP_TIMEOUT`. A failure, panic, or timeout is
+reported, the remaining hooks still run, and a test that otherwise passed
+fails. A killed test process skips cleanup, so names are unique to the run.
 
 ## What a test can do
 
@@ -54,7 +72,9 @@ test creates, such as providers, in the test itself.
   up. `cx.adopt(name)` returns a `Sandbox` handle for one the CLI created.
 - `cx.expect_listed` and `cx.expect_absent` to check a sandbox in list and get.
 - `cx.cli([...])` to run the installed CLI against the same gateway, with
-  `success()`, `failure()`, and `json()` on the output.
+  `success()`, `failure()`, and `json()` on the output. `cx.cli_handle()`
+  returns that CLI for code that can't borrow `cx`, such as a cleanup hook.
+- `cx.on_cleanup(name, hook)` to remove something when the test ends.
 - `cx.client()` for SDK calls testkit does not wrap.
 - `poll_until` for waiting on other state.
 

tests/testkit/testkit/Cargo.toml

@@ -24,3 +24,6 @@ rand = "0.9"
 serde = "1"
 serde_json = "1"
 tokio = { version = "1.43", features = ["macros", "process", "rt", "time"] }
+
+[dev-dependencies]
+tonic = "0.14"