test: stop integration suites failing on container teardown timeout (#466)

* test: stop integration suites failing on container teardown timeout

Recent CI failures on green code (MariaDB twice, postgres-ssh once) all
read "Hook timed out in 10000ms" at the suite's afterAll. Teardown is
Testcontainers' stop(), i.e. `docker stop -t 0` + `docker rm -v`. The
suites share one Docker daemon, and while Oracle (1.2GB) or SQL Server is
still pulling/extracting its image, removing a sibling container with its
data volume can exceed Vitest's default 10s hookTimeout. Every beforeAll
already carries an explicit 60-360s timeout; afterAll relied on the
default.

- vitest.config.ts: hookTimeout 60s for the integration project.
- IntegrationTestBase.cleanup(): stop the container in a finally so a
  disconnect failure can't leak a running container.
- testing skill: document the symptom and cause.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01TZPfFnRTrHzHhsCzjrZjgM

* test: narrow the cleanup comment to what finally guarantees

A pending disconnect() never reaches the finally, so the container-stop
guarantee covers thrown errors, not hangs; say so.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01TZPfFnRTrHzHhsCzjrZjgM

---------

Co-authored-by: Claude <[email protected]>

Author: tianzhou

.claude/skills/testing/SKILL.md

@@ -145,6 +145,16 @@ docker ps -a | grep testcontainers  # Find leaked containers
 docker container prune               # Clean up stopped containers
 ```
 
+### "Hook timed out in 10000ms" in `afterAll`
+All tests pass but the suite is marked failed at its `afterAll` (seen on MariaDB and
+postgres-ssh in CI). Teardown is `container.stop()`, which runs `docker stop -t 0`
+followed by `docker rm -v` (removing the data volume). On a shared Docker daemon that
+is simultaneously pulling/extracting the Oracle or SQL Server image, that removal can
+exceed Vitest's 10s default hook timeout. The integration project sets
+`hookTimeout: 60_000` in `vitest.config.ts` for this reason — do not pass a shorter
+timeout to `afterAll`, and if you add a suite with its own teardown, let it use the
+project default rather than the Vitest default.
+
 ### CI Failures
 The CI workflow (`.github/workflows/run-tests.yml`) runs unit and integration tests as separate parallel jobs on PR events. Unit tests don't need Docker; integration tests verify Docker availability first. Check the specific job that failed.
 

src/connectors/__tests__/shared/integration-test-base.ts

@@ -79,11 +79,18 @@ export abstract class IntegrationTestBase<TContainer extends TestContainer> {
    * Cleanup method to be called in afterAll
    */
   async cleanup(): Promise<void> {
-    if (this.connector) {
-      await this.connector.disconnect();
-    }
-    if (this.container) {
-      await this.container.stop();
+    // Stop the container even if disconnecting throws: a connector error
+    // here must not leak a running container into the next suite (or onto
+    // the CI runner). A disconnect that never settles is bounded only by the
+    // integration project's hookTimeout, not by this block.
+    try {
+      if (this.connector) {
+        await this.connector.disconnect();
+      }
+    } finally {
+      if (this.container) {
+        await this.container.stop();
+      }
     }
   }
 

vitest.config.ts

@@ -22,6 +22,14 @@ export default defineConfig({
         test: {
           name: 'integration',
           include: ['src/**/*integration*.test.ts'],
+          // Container teardown (`docker stop` + `docker rm -v`) shares one Docker
+          // daemon with every other suite in the run. While Oracle or SQL Server
+          // is still pulling/extracting its image, removing a sibling container
+          // and its data volume can take well over Vitest's 10s default, and the
+          // `afterAll` hooks in these suites fail with "Hook timed out in 10000ms"
+          // even though every test passed. Setup hooks keep their explicit
+          // per-suite timeouts; this raises the floor for teardown.
+          hookTimeout: 60_000,
         },
       },
     ],