Skip to content

Commit 2260a05

Browse files
committed
Improve Docker cleanup in tests to reduce other flakes & races
1 parent fb1a011 commit 2260a05

File tree

3 files changed

+52
-6
lines changed

3 files changed

+52
-6
lines changed

test/integration/interceptors/docker-attachment.spec.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ import { delay } from '@httptoolkit/util';
88
import Docker from 'dockerode';
99
import fetch from 'node-fetch';
1010

11-
import { FIXTURES_DIR } from '../../test-util';
11+
import { FIXTURES_DIR, TEST_CONTAINER_LABEL, removeTestContainers } from '../../test-util';
1212
import { setupInterceptor, itIsAvailable } from './interceptor-test-utils';
1313
import { waitForDockerStream } from '../../../src/interceptors/docker/docker-utils';
14+
import { DOCKER_CONTAINER_LABEL } from '../../../src/interceptors/docker/docker-commands';
1415

1516
const docker = new Docker();
1617
const DOCKER_FIXTURES = path.join(FIXTURES_DIR, 'docker');
@@ -49,6 +50,7 @@ async function buildAndRun(dockerFolder: string, options: {
4950
// Run the container, using its default entrypoint
5051
const container = await docker.createContainer({
5152
Image: imageName,
53+
Labels: { [TEST_CONTAINER_LABEL]: '' },
5254
HostConfig: {
5355
AutoRemove: true,
5456
PortBindings: portBindings
@@ -86,10 +88,15 @@ async function buildAndRun(dockerFolder: string, options: {
8688
}
8789

8890
async function killAllContainers() {
89-
const containers = await docker.listContainers()
91+
const containers = await docker.listContainers({
92+
filters: JSON.stringify({ label: [TEST_CONTAINER_LABEL] })
93+
});
9094
await Promise.all(containers.map(c =>
91-
docker.getContainer(c.Id).stop({ t: 0 })
95+
docker.getContainer(c.Id).stop({ t: 0 }).catch(() => {})
9296
));
97+
98+
// Wait until containers are fully gone (AutoRemove may be async):
99+
await removeTestContainers(docker, TEST_CONTAINER_LABEL);
93100
}
94101

95102
const interceptorSetup = setupInterceptor('docker-attach');
@@ -105,6 +112,7 @@ describe('Docker single-container interceptor', function () {
105112
const { server, interceptor } = await interceptorSetup;
106113
await killAllContainers();
107114
await interceptor.deactivate(server.port);
115+
await removeTestContainers(docker, DOCKER_CONTAINER_LABEL);
108116
await server.stop();
109117
});
110118

test/integration/interceptors/docker-terminal-interception.spec.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { expect } from 'chai';
88
import { delay } from '@httptoolkit/util';
99

1010
import { setupTest } from './interceptor-test-utils';
11-
import { FIXTURES_DIR } from '../../test-util';
11+
import { FIXTURES_DIR, TEST_CONTAINER_LABEL, removeTestContainers } from '../../test-util';
1212
import { spawnToResult } from '../../../src/util/process-management';
1313

1414
import { getTerminalEnvVars } from '../../../src/interceptors/terminal/terminal-env-overrides';
@@ -17,6 +17,7 @@ import {
1717
startDockerInterceptionServices,
1818
stopDockerInterceptionServices
1919
} from '../../../src/interceptors/docker/docker-interception-services';
20+
import { DOCKER_CONTAINER_LABEL } from '../../../src/interceptors/docker/docker-commands';
2021

2122
const testSetup = setupTest();
2223

@@ -37,6 +38,13 @@ describe('Docker CLI interception', function () {
3738
afterEach(async () => {
3839
const { server } = await testSetup;
3940
await stopDockerInterceptionServices(server.port, ruleParams);
41+
42+
// Make sure all relevant containers are fully gone, so the next test
43+
// definitely starts clean:
44+
const docker = new Docker();
45+
await removeTestContainers(docker, TEST_CONTAINER_LABEL);
46+
await removeTestContainers(docker, DOCKER_CONTAINER_LABEL);
47+
4048
await server.stop();
4149
});
4250

@@ -318,7 +326,7 @@ Successfully built <hash>
318326

319327
const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env);
320328

321-
const uninterceptedResult = await spawnToResult('docker', ['create', 'node:14']);
329+
const uninterceptedResult = await spawnToResult('docker', ['create', '--label', TEST_CONTAINER_LABEL, 'node:14']);
322330

323331
expect(uninterceptedResult.exitCode).to.equal(0);
324332
expect(uninterceptedResult.stderr).to.equal('');

test/test-util.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,33 @@
11
import * as path from 'path';
22

3-
export const FIXTURES_DIR = path.join(__dirname, 'fixtures');
3+
import { delay } from '@httptoolkit/util';
4+
import Docker from 'dockerode';
5+
6+
export const FIXTURES_DIR = path.join(__dirname, 'fixtures');
7+
8+
// Label applied to all Docker containers created by tests, to allow targeted cleanup
9+
// without interfering with system containers or the app's own labels (tech.httptoolkit.docker.*).
10+
export const TEST_CONTAINER_LABEL = 'tech.httptoolkit.test';
11+
12+
// Remove all containers matching a label filter, polling until they're fully
13+
// gone from Docker's perspective (not just API-removed but still visible).
14+
export async function removeTestContainers(docker: Docker, label: string) {
15+
const containers = await docker.listContainers({
16+
all: true,
17+
filters: JSON.stringify({ label: [label] })
18+
});
19+
20+
await Promise.all(containers.map(c =>
21+
docker.getContainer(c.Id).remove({ force: true }).catch(() => {})
22+
));
23+
24+
// Poll until Docker confirms no matching containers remain:
25+
for (let i = 0; i < 20; i++) {
26+
const remaining = await docker.listContainers({
27+
all: true,
28+
filters: JSON.stringify({ label: [label] })
29+
});
30+
if (remaining.length === 0) return;
31+
await delay(100);
32+
}
33+
}

0 commit comments

Comments
 (0)