Skip to content

Commit 9237a7a

Browse files
authored
chore(cli-integ): add per-test timeouts (#24504)
We couldn't use the jest timeout feature for our integration tests, and ended up running without timeouts. This made it EXTREMELY hard to debug an issue where the tests ended up not finishing, and not producing any output to indicate why. Re-add the ability to have timeouts on test, and set a timeout of 10 minutes on all of them. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent e39bdea commit 9237a7a

File tree

5 files changed

+42
-6
lines changed

5 files changed

+42
-6
lines changed

packages/@aws-cdk-testing/cli-integ/lib/resource-pool.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,14 @@ export class ResourcePool<A extends string=string> {
8787

8888
private makeLease(value: A): ILease<A> {
8989
let disposed = false;
90-
process.stderr.write(`Lease acquired by ${process.pid}: ${value}`);
9190
return {
9291
value,
9392
dispose: async () => {
9493
if (disposed) {
9594
throw new Error('Calling dispose() on an already-disposed lease.');
9695
}
9796
disposed = true;
98-
return this.returnValue(value).finally(() => process.stderr.write(`Lease returned by ${process.pid}: ${value}`));
97+
return this.returnValue(value);
9998
},
10099
};
101100
}

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import { packageSourceInSubprocess } from './package-sources/subprocess';
99
import { RESOURCES_DIR } from './resources';
1010
import { shell, ShellOptions, ShellHelper, rimraf } from './shell';
1111
import { AwsContext, withAws } from './with-aws';
12+
import { withTimeout } from './with-timeout';
13+
14+
export const DEFAULT_TEST_TIMEOUT_S = 10 * 60;
1215

1316
/**
1417
* Higher order function to execute a block with a CDK app fixture
@@ -135,7 +138,7 @@ export function withMonolithicCfnIncludeCdkApp<A extends TestContext>(block: (co
135138
* test declaration but centralizing it is going to make it convenient to modify in the future.
136139
*/
137140
export function withDefaultFixture(block: (context: TestFixture) => Promise<void>) {
138-
return withAws(withCdkApp(block));
141+
return withAws(withTimeout(DEFAULT_TEST_TIMEOUT_S, withCdkApp(block)));
139142
}
140143

141144
export interface DisableBootstrapContext {

packages/@aws-cdk-testing/cli-integ/lib/with-sam.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ import { TestContext } from './integ-test';
77
import { RESOURCES_DIR } from './resources';
88
import { ShellOptions, rimraf } from './shell';
99
import { AwsContext, withAws } from './with-aws';
10-
import { cloneDirectory, installNpmPackages, TestFixture } from './with-cdk-app';
10+
import { cloneDirectory, installNpmPackages, TestFixture, DEFAULT_TEST_TIMEOUT_S } from './with-cdk-app';
11+
import { withTimeout } from './with-timeout';
1112

1213

1314
export interface ActionOutput {
@@ -113,7 +114,7 @@ function errorCausedByGoPkg(error: string) {
113114
* SAM Integration test fixture for CDK - SAM integration test cases
114115
*/
115116
export function withSamIntegrationFixture(block: (context: SamIntegrationTestFixture) => Promise<void>) {
116-
return withAws(withSamIntegrationCdkApp(block));
117+
return withAws(withTimeout(DEFAULT_TEST_TIMEOUT_S, withSamIntegrationCdkApp(block)));
117118
}
118119

119120
export class SamIntegrationTestFixture extends TestFixture {
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* Run a block with a timeout
3+
*
4+
* We can't use the jest timeout feature:
5+
*
6+
* - `jest.concurrent()` does not do any concurrency management. It starts all
7+
* tests at the same time.
8+
* - Our tests use locking to make sure only one test is running at a time per
9+
* region.
10+
*
11+
* The wait time for the locks is included in the jest test timeout. We therefore
12+
* need to set it unreasonably high (as long as the last test may need to wait
13+
* if all tests are executed using only 1 region, and they effectively execute
14+
* sequentially), which makes it not useful to detect stuck tests.
15+
*
16+
* The `withTimeout()` modifier makes it possible to measure only a specific
17+
* block of code. In our case: the effective test code, excluding the wait time.
18+
*/
19+
export function withTimeout<A>(seconds: number, block: (x: A) => Promise<void>) {
20+
return (x: A) => {
21+
const timeOut = new Promise<void>((_ok, ko) => {
22+
const timerHandle = setTimeout(
23+
() => ko(new Error(`Timeout: test took more than ${seconds}s to complete`)),
24+
seconds * 1000);
25+
timerHandle.unref();
26+
});
27+
28+
return Promise.race([
29+
block(x),
30+
timeOut,
31+
]);
32+
};
33+
}

packages/@aws-cdk-testing/cli-integ/test/resource-pool.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ test('double dispose throws', async () => {
3232
const lease = await pool.take();
3333

3434
await lease.dispose();
35-
expect(() => lease.dispose()).toThrow();
35+
await expect(() => lease.dispose()).rejects.toThrow();
3636
});
3737

3838
test('somewhat balance', async () => {

0 commit comments

Comments
 (0)