Skip to content

Commit 5e2872d

Browse files
emily-shenemily-shen
authored
improve port checking (#77)
* fix: check ports in startAndWaitForPorts even if the container has already started * feat: add waitForPort() and refactor startAndWaitForPorts() accordingly. also make sure user-provided waitInterval is actually being used * and tests
1 parent 66cfba1 commit 5e2872d

File tree

12 files changed

+476
-132
lines changed

12 files changed

+476
-132
lines changed

‎.github/workflows/tests.yml

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,18 @@ jobs:
2121
node-version: '20'
2222
cache: 'npm'
2323

24-
- name: Install dependencies
25-
working-directory: fixtures/basic
26-
run: npm install
27-
28-
- name: Run integration tests
29-
working-directory: fixtures/basic
30-
run: npm run test
24+
- name: Install dependencies and run tests for all fixtures
25+
run: |
26+
for fixture in fixtures/*/; do
27+
if [ -f "$fixture/package.json" ]; then
28+
echo "Installing dependencies for $fixture"
29+
cd "$fixture"
30+
npm install
31+
echo "Running tests for $fixture"
32+
npm run test
33+
cd - > /dev/null
34+
fi
35+
done
3136
timeout-minutes: 10
3237

3338

@@ -47,6 +52,13 @@ jobs:
4752
- name: Install dependencies
4853
run: npm ci
4954

50-
- name: Type check fixtures
51-
working-directory: fixtures/basic
52-
run: npx tsc --noEmit
55+
- name: Type check all fixtures
56+
run: |
57+
for fixture in fixtures/*/; do
58+
if [ -f "$fixture/tsconfig.json" ]; then
59+
echo "Type checking $fixture"
60+
cd "$fixture"
61+
npx tsc --noEmit
62+
cd - > /dev/null
63+
fi
64+
done

‎fixtures/helpers/index.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export class WranglerDevRunner {
3232
this.process.stdout.on('data', (data: Buffer) => {
3333
const output = data.toString();
3434
this.stdout += output;
35+
console.log(output);
3536

3637
// Check for ready pattern
3738
const match = output.match(/Ready on (?<url>https?:\/\/.*)/);
@@ -44,6 +45,7 @@ export class WranglerDevRunner {
4445

4546
this.process.stderr.on('data', (data: Buffer) => {
4647
this.stderr += data.toString();
48+
console.log(data.toString());
4749
});
4850
});
4951
}
@@ -64,7 +66,8 @@ export class WranglerDevRunner {
6466
for (const id of containerId) {
6567
await fetch(this.url + '/stop?id=' + id);
6668
}
67-
69+
// give it a second to run the onStop hook before we kill the process
70+
await new Promise(resolve => setTimeout(resolve, 1000));
6871
this.process.kill('SIGTERM');
6972

7073
// Wait a bit for the process to finish

‎fixtures/multiple-ports/Dockerfile

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# syntax=docker/dockerfile:1
2+
FROM node:22-alpine
3+
4+
WORKDIR /usr/src/app
5+
6+
RUN echo '{"name": "test-container", "version": "1.0.0"}' > package.json
7+
8+
COPY container_src/server.js server.js
9+
10+
EXPOSE 8080 8081
11+
STOPSIGNAL SIGINT

‎fixtures/multiple-ports/container_src/server.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { createServer } from "http";
2+
import { setTimeout } from "timers/promises";
3+
4+
const server = createServer(function (req, res) {
5+
if (req.url === '/error') {
6+
res.writeHead(500, { "Content-Type": "text/plain" });
7+
res.end("Internal server error");
8+
return;
9+
}
10+
11+
res.writeHead(200, { "Content-Type": "text/plain" });
12+
res.end(`Hello from test container server one! process.env.MESSAGE: ${process.env.MESSAGE}`);
13+
});
14+
15+
server.listen(8080, function () {
16+
console.log(`Test server listening on port 8080`);
17+
});
18+
server.on("exit", () => {
19+
console.log("Test server one exiting");
20+
})
21+
22+
23+
24+
await setTimeout(5000)
25+
26+
27+
const server2 = createServer(function (req, res) {
28+
if (req.url === '/error') {
29+
res.writeHead(500, { "Content-Type": "text/plain" });
30+
res.end("Internal server error");
31+
return;
32+
}
33+
34+
res.writeHead(200, { "Content-Type": "text/plain" });
35+
res.end(`Hello from test container server two! process.env.MESSAGE: ${process.env.MESSAGE}`);
36+
});
37+
server2.listen(8081, function () {
38+
console.log(`Test server two listening on port 8081`);
39+
});
40+
41+
server2.on("exit", function () {
42+
console.log("Test server two exiting");
43+
});
44+
45+

‎fixtures/multiple-ports/package.json

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"name": "containers-test-fixture",
3+
"version": "1.0.0",
4+
"description": "Integration test fixture for @cloudflare/containers",
5+
"type": "module",
6+
"scripts": {
7+
"test": "vitest run",
8+
"test:watch": "vitest",
9+
"test:local": "vitest run -t 'Local Instance Tests'",
10+
"test:deployed": "vitest run -t 'Deployed Instance Tests'",
11+
"dev": "wrangler dev",
12+
"deploy": "wrangler deploy",
13+
"build": "tsc"
14+
},
15+
"devDependencies": {
16+
"@types/node": "^20.0.0",
17+
"typescript": "^5.0.0",
18+
"vitest": "^1.0.0",
19+
"wrangler": "https://pkg.pr.new/wrangler@main"
20+
},
21+
"dependencies": {
22+
"@cloudflare/workers-types": "^4.0.0"
23+
}
24+
}

‎fixtures/multiple-ports/src/index.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
import { Container } from '../../../src/lib/container';
2+
import { getContainer, getRandom, switchPort } from '../../../src/lib/utils';
3+
4+
/**
5+
* Test Container implementation for integration testing
6+
*/
7+
export class TestContainer extends Container {
8+
defaultPort = 8080;
9+
10+
// Set how long the container should stay active without requests
11+
sleepAfter = '3m';
12+
13+
constructor(ctx: any, env: any) {
14+
super(ctx, env);
15+
16+
// Set container configuration
17+
this.envVars = {
18+
MESSAGE: 'default message',
19+
};
20+
this.entrypoint = ['node', 'server.js'];
21+
}
22+
23+
override async onStart(): Promise<void> {
24+
console.log('onStart hook called');
25+
}
26+
27+
override async onStop(): Promise<void> {
28+
console.log('onStop hook called');
29+
}
30+
31+
override onError(error: unknown): any {
32+
console.log('onError hook called with error:', error);
33+
throw error;
34+
}
35+
}
36+
37+
export default {
38+
async fetch(
39+
request: Request,
40+
env: { CONTAINER: DurableObjectNamespace<TestContainer> }
41+
): Promise<Response> {
42+
const url = new URL(request.url);
43+
// get a new container instance per request
44+
45+
const id = url.searchParams.get('id') || 'singleton';
46+
const container = getContainer(env.CONTAINER, id);
47+
48+
if (url.pathname === '/startAndWaitFor8080') {
49+
// waits for default port 8080 only
50+
await container.startAndWaitForPorts({
51+
startOptions: {
52+
envVars: { MESSAGE: 'start with startAndWaitForPorts' },
53+
},
54+
});
55+
return new Response('start request sent waiting for 8080');
56+
}
57+
if (url.pathname === '/startAndWaitForAllPorts') {
58+
// waits for default port 8080 only
59+
await container.startAndWaitForPorts({
60+
startOptions: {
61+
envVars: { MESSAGE: 'start with startAndWaitForPorts' },
62+
},
63+
ports: [8080, 8081],
64+
});
65+
return new Response('start request sent waiting for all ports');
66+
}
67+
if (url.pathname === '/server-one') {
68+
return container.containerFetch(request, 8080);
69+
}
70+
if (url.pathname === '/server-two') {
71+
return container.containerFetch(request, 8081);
72+
}
73+
if (url.pathname === '/start') {
74+
await container.start({
75+
envVars: { MESSAGE: 'start with start' },
76+
});
77+
return new Response('start request sent');
78+
}
79+
if (url.pathname === '/waitForPort') {
80+
const portToCheck = parseInt(url.searchParams.get('port') || '8080');
81+
await container.waitForPort({ portToCheck });
82+
return new Response('port');
83+
}
84+
85+
if (url.pathname === '/stop') {
86+
await container.destroy();
87+
return new Response('Container stopping');
88+
}
89+
return new Response('Not Found');
90+
},
91+
};

‎fixtures/multiple-ports/test/index.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { WranglerDevRunner } from '../../helpers/index.js';
2+
import { test, expect, describe } from 'vitest';
3+
import { randomUUID } from 'node:crypto';
4+
5+
const fetchOrTimeout = async (fetchPromise: Promise<Response>, timeoutMs: number) => {
6+
const timeoutPromise = new Promise((_, reject) =>
7+
setTimeout(() => reject(new Error('timeout')), timeoutMs)
8+
);
9+
return await Promise.race([fetchPromise, timeoutPromise]);
10+
};
11+
12+
describe('multiple ports functionality', () => {
13+
test('waitForPort should wait for port to be ready before returning', async () => {
14+
const runner = new WranglerDevRunner();
15+
const url = await runner.getUrl();
16+
const id = randomUUID();
17+
18+
// This only waits for the default port 8080 to be ready
19+
await fetch(`${url}/startAndWaitFor8080?id=${id}`);
20+
21+
// first server should be ready immediately
22+
let fetchPromise = fetch(`${url}/server-one?id=${id}`);
23+
let res = (await fetchOrTimeout(fetchPromise, 1000)) as Response;
24+
expect(await res.text()).toBe(
25+
'Hello from test container server one! process.env.MESSAGE: start with startAndWaitForPorts'
26+
);
27+
28+
// Second server listening on 8081 should not be ready yet - this call should not resolve within 1s
29+
fetchPromise = fetch(`${url}/server-two?id=${id}`);
30+
expect(fetchOrTimeout(fetchPromise, 1000)).rejects.toThrow('timeout');
31+
32+
// Wait for the default port (8080) to be ready
33+
34+
await fetch(`${url}/waitForPort?id=${id}&port=8081`);
35+
36+
// Verify the container is actually ready by making a request
37+
fetchPromise = fetch(`${url}/server-two?id=${id}`);
38+
res = (await fetchOrTimeout(fetchPromise, 1000)) as Response;
39+
expect(await res.text()).toBe(
40+
'Hello from test container server two! process.env.MESSAGE: start with startAndWaitForPorts'
41+
);
42+
43+
await runner.stop([id]);
44+
});
45+
46+
test('startAndWaitForPorts should wait for all specified ports to be ready', async () => {
47+
const runner = new WranglerDevRunner();
48+
const url = await runner.getUrl();
49+
const id = randomUUID();
50+
51+
// Start and wait for ports should wait for all ports to be ready
52+
await fetch(`${url}/startAndWaitForAllPorts?id=${id}`);
53+
54+
// first server should be ready immediately
55+
let fetchPromise = fetch(`${url}/server-one?id=${id}`);
56+
let res = (await fetchOrTimeout(fetchPromise, 1000)) as Response;
57+
expect(await res.text()).toBe(
58+
'Hello from test container server one! process.env.MESSAGE: start with startAndWaitForPorts'
59+
);
60+
61+
// second server should be ready immediately
62+
fetchPromise = fetch(`${url}/server-two?id=${id}`);
63+
res = (await fetchOrTimeout(fetchPromise, 1000)) as Response;
64+
expect(await res.text()).toBe(
65+
'Hello from test container server two! process.env.MESSAGE: start with startAndWaitForPorts'
66+
);
67+
68+
await runner.stop([id]);
69+
});
70+
});

‎fixtures/multiple-ports/tsconfig.json

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
{
2+
"compilerOptions": {
3+
/* Visit https://aka.ms/tsconfig.json to read more about this file */
4+
5+
/* Set the JavaScript language version for emitted JavaScript and include compatible library declarations. */
6+
"target": "es2021",
7+
/* Specify a set of bundled library declaration files that describe the target runtime environment. */
8+
"lib": ["es2021"],
9+
/* Specify what JSX code is generated. */
10+
"jsx": "react-jsx",
11+
12+
/* Specify what module code is generated. */
13+
"module": "es2022",
14+
/* Specify how TypeScript looks up a file from a given module specifier. */
15+
"moduleResolution": "Bundler",
16+
/* Specify type package names to be included without being referenced in a source file. */
17+
"types": ["@cloudflare/workers-types/2023-07-01"],
18+
/* Enable importing .json files */
19+
"resolveJsonModule": true,
20+
21+
/* Allow JavaScript files to be a part of your program. Use the `checkJS` option to get errors from these files. */
22+
"allowJs": true,
23+
/* Enable error reporting in type-checked JavaScript files. */
24+
"checkJs": false,
25+
26+
/* Disable emitting files from a compilation. */
27+
"noEmit": true,
28+
29+
/* Ensure that each file can be safely transpiled without relying on other imports. */
30+
"isolatedModules": true,
31+
/* Allow 'import x from y' when a module doesn't have a default export. */
32+
"allowSyntheticDefaultImports": true,
33+
/* Ensure that casing is correct in imports. */
34+
"forceConsistentCasingInFileNames": true,
35+
36+
/* Enable all strict type-checking options. */
37+
"strict": true,
38+
39+
/* Skip type checking all .d.ts files. */
40+
"skipLibCheck": true
41+
},
42+
"exclude": ["test"],
43+
"include": ["worker-configuration.d.ts", "src/**/*.ts"]
44+
}

‎fixtures/multiple-ports/vitest.config.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { defineConfig } from 'vitest/config';
2+
3+
export default defineConfig({
4+
test: {
5+
globals: true,
6+
testTimeout: 60000,
7+
hookTimeout: 120000,
8+
},
9+
});

0 commit comments

Comments
 (0)