Skip to content

Commit b548d5d

Browse files
mrgrainmrgrain
committed
fix(integ-runner): improve git directory handling for snapshot operations
Fixes issues with git operations by using the -C flag to specify the correct git repository directory. This ensures integ-runner works correctly when executed from outside the repository under test. Also standardizes the directory used for running CDK apps to always be the current working directory.
1 parent 779352d commit b548d5d

File tree

5 files changed

+48
-51
lines changed

5 files changed

+48
-51
lines changed

‎packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -104,24 +104,19 @@ export class IntegTest {
104104

105105
constructor(public readonly info: IntegTestInfo) {
106106
this.appCommand = info.appCommand ?? 'node {filePath}';
107-
this.absoluteFileName = path.resolve(info.fileName);
108-
this.fileName = path.relative(process.cwd(), info.fileName);
109107

110-
const parsed = path.parse(this.fileName);
108+
// for consistency, always run the CDK apps under test from the CWD
109+
// this is especially important for languages that use the CWD to discover assets
110+
// @see https://github.com/aws/aws-cdk-cli/issues/638
111+
this.directory = process.cwd();
112+
this.absoluteFileName = path.resolve(info.fileName);
113+
this.fileName = path.relative(this.directory, info.fileName);
111114
this.discoveryRelativeFileName = path.relative(info.discoveryRoot, info.fileName);
112-
// if `--watch` then we need the directory to be the cwd
113-
this.directory = info.watch ? process.cwd() : parsed.dir;
114-
115-
// if we are running in a package directory then just use the fileName
116-
// as the testname, but if we are running in a parent directory with
117-
// multiple packages then use the directory/filename as the testname
118-
//
119-
// Looks either like `integ.mytest` or `package/test/integ.mytest`.
120-
const relDiscoveryRoot = path.relative(process.cwd(), info.discoveryRoot);
121-
this.testName = this.directory === path.join(relDiscoveryRoot, 'test') || this.directory === path.join(relDiscoveryRoot)
122-
? parsed.name
123-
: path.join(path.relative(this.info.discoveryRoot, parsed.dir), parsed.name);
124115

116+
// We treat the discovery root as the base for display names
117+
// Looks either like `integ.mytest` or `package/test/integ.mytest`
118+
const parsed = path.parse(this.fileName);
119+
this.testName = path.join(path.relative(this.info.discoveryRoot, parsed.dir), parsed.name);
125120
this.normalizedTestName = parsed.name;
126121
this.snapshotDir = path.join(parsed.dir, `${parsed.base}.snapshot`);
127122
this.temporaryOutputDir = path.join(parsed.dir, `${CDK_OUTDIR_PREFIX}.${parsed.base}.snapshot`);

‎packages/@aws-cdk/integ-runner/test/helpers.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,16 @@ export class MockCdkProvider {
7878
diagnostics: Diagnostic[];
7979
destructiveChanges: DestructiveChange[];
8080
}> {
81+
const actualSnapshotLocation = actualSnapshot ? 'test/test-data/' + actualSnapshot : undefined;
82+
8183
// WHEN
8284
const integTest = new IntegSnapshotRunner({
8385
cdk: this.cdk,
8486
test: new IntegTest({
8587
fileName: 'test/test-data/' + integTestFile,
8688
discoveryRoot: 'test/test-data',
8789
}),
88-
integOutDir: actualSnapshot ? 'test/test-data/' + actualSnapshot : undefined,
90+
integOutDir: actualSnapshotLocation,
8991
});
9092

9193
const results = await integTest.testSnapshot();
@@ -98,8 +100,8 @@ export class MockCdkProvider {
98100
CDK_INTEG_REGION: 'test-region',
99101
}),
100102
context: expect.any(Object),
101-
execCmd: ['node', integTestFile],
102-
output: actualSnapshot ?? `cdk-integ.out.${integTestFile}.snapshot`,
103+
execCmd: ['node', 'test/test-data/' + integTestFile],
104+
output: actualSnapshotLocation ?? `test/test-data/cdk-integ.out.${integTestFile}.snapshot`,
103105
});
104106

105107
return results;

‎packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ describe('IntegTest runIntegTests', () => {
6161
expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
6262
expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
6363
expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({
64-
app: 'xxxxx.test-with-snapshot.js.snapshot',
64+
app: 'test/test-data/xxxxx.test-with-snapshot.js.snapshot',
6565
requireApproval: 'never',
6666
pathMetadata: false,
6767
assetMetadata: false,
@@ -76,11 +76,11 @@ describe('IntegTest runIntegTests', () => {
7676
stacks: ['test-stack'],
7777
});
7878
expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({
79-
app: 'node xxxxx.test-with-snapshot.js',
79+
app: 'node test/test-data/xxxxx.test-with-snapshot.js',
8080
requireApproval: 'never',
8181
pathMetadata: false,
8282
assetMetadata: false,
83-
output: 'cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot',
83+
output: 'test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot',
8484
profile: undefined,
8585
context: expect.not.objectContaining({
8686
'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({
@@ -92,7 +92,7 @@ describe('IntegTest runIntegTests', () => {
9292
stacks: ['test-stack', 'new-test-stack'],
9393
});
9494
expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({
95-
app: 'node xxxxx.test-with-snapshot.js',
95+
app: 'node test/test-data/xxxxx.test-with-snapshot.js',
9696
pathMetadata: false,
9797
assetMetadata: false,
9898
context: expect.not.objectContaining({
@@ -104,7 +104,7 @@ describe('IntegTest runIntegTests', () => {
104104
profile: undefined,
105105
force: true,
106106
all: true,
107-
output: 'cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot',
107+
output: 'test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot',
108108
});
109109
});
110110

@@ -126,7 +126,7 @@ describe('IntegTest runIntegTests', () => {
126126
expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
127127
expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
128128
expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({
129-
app: 'node xxxxx.integ-test1.js',
129+
app: 'node test/test-data/xxxxx.integ-test1.js',
130130
requireApproval: 'never',
131131
pathMetadata: false,
132132
assetMetadata: false,
@@ -137,17 +137,17 @@ describe('IntegTest runIntegTests', () => {
137137
}),
138138
lookups: false,
139139
stacks: ['stack1'],
140-
output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot',
140+
output: 'test/test-data/cdk-integ.out.xxxxx.integ-test1.js.snapshot',
141141
});
142142
expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({
143-
app: 'node xxxxx.integ-test1.js',
143+
app: 'node test/test-data/xxxxx.integ-test1.js',
144144
pathMetadata: false,
145145
assetMetadata: false,
146146
versionReporting: false,
147147
context: expect.any(Object),
148148
force: true,
149149
all: true,
150-
output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot',
150+
output: 'test/test-data/cdk-integ.out.xxxxx.integ-test1.js.snapshot',
151151
});
152152
});
153153

@@ -169,7 +169,7 @@ describe('IntegTest runIntegTests', () => {
169169
expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
170170
expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
171171
expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({
172-
app: 'node xxxxx.test-with-snapshot-assets-diff.js',
172+
app: 'node test/test-data/xxxxx.test-with-snapshot-assets-diff.js',
173173
requireApproval: 'never',
174174
pathMetadata: false,
175175
assetMetadata: false,
@@ -181,11 +181,11 @@ describe('IntegTest runIntegTests', () => {
181181
versionReporting: false,
182182
lookups: true,
183183
stacks: ['test-stack'],
184-
output: 'cdk-integ.out.xxxxx.test-with-snapshot-assets-diff.js.snapshot',
184+
output: 'test/test-data/cdk-integ.out.xxxxx.test-with-snapshot-assets-diff.js.snapshot',
185185
profile: undefined,
186186
});
187187
expect(cdkMock.mocks.synthFast).toHaveBeenCalledWith({
188-
execCmd: ['node', 'xxxxx.test-with-snapshot-assets-diff.js'],
188+
execCmd: ['node', 'test/test-data/xxxxx.test-with-snapshot-assets-diff.js'],
189189
env: expect.objectContaining({
190190
CDK_INTEG_ACCOUNT: '12345678',
191191
CDK_INTEG_REGION: 'test-region',
@@ -195,10 +195,10 @@ describe('IntegTest runIntegTests', () => {
195195
vpcId: 'vpc-60900905',
196196
}),
197197
}),
198-
output: 'xxxxx.test-with-snapshot-assets-diff.js.snapshot',
198+
output: 'test/test-data/xxxxx.test-with-snapshot-assets-diff.js.snapshot',
199199
});
200200
expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({
201-
app: 'node xxxxx.test-with-snapshot-assets-diff.js',
201+
app: 'node test/test-data/xxxxx.test-with-snapshot-assets-diff.js',
202202
pathMetadata: false,
203203
assetMetadata: false,
204204
context: expect.not.objectContaining({
@@ -209,7 +209,7 @@ describe('IntegTest runIntegTests', () => {
209209
versionReporting: false,
210210
force: true,
211211
all: true,
212-
output: 'cdk-integ.out.xxxxx.test-with-snapshot-assets-diff.js.snapshot',
212+
output: 'test/test-data/cdk-integ.out.xxxxx.test-with-snapshot-assets-diff.js.snapshot',
213213
});
214214
});
215215

@@ -231,20 +231,20 @@ describe('IntegTest runIntegTests', () => {
231231
expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
232232
expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
233233
expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(1, expect.objectContaining({
234-
app: 'xxxxx.test-with-snapshot.js.snapshot',
234+
app: 'test/test-data/xxxxx.test-with-snapshot.js.snapshot',
235235
context: expect.any(Object),
236236
stacks: ['test-stack'],
237237
}));
238238
expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(2, expect.not.objectContaining({
239239
rollback: false,
240240
}));
241241
expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(3, expect.objectContaining({
242-
app: 'node xxxxx.test-with-snapshot.js',
242+
app: 'node test/test-data/xxxxx.test-with-snapshot.js',
243243
stacks: ['Bundling/DefaultTest/DeployAssert'],
244244
rollback: false,
245245
}));
246246
expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({
247-
app: 'node xxxxx.test-with-snapshot.js',
247+
app: 'node test/test-data/xxxxx.test-with-snapshot.js',
248248
pathMetadata: false,
249249
assetMetadata: false,
250250
context: expect.not.objectContaining({
@@ -255,7 +255,7 @@ describe('IntegTest runIntegTests', () => {
255255
versionReporting: false,
256256
force: true,
257257
all: true,
258-
output: 'cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot',
258+
output: 'test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot',
259259
});
260260
});
261261

@@ -313,8 +313,8 @@ describe('IntegTest runIntegTests', () => {
313313
// THEN
314314
expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
315315
expect(cdkMock.mocks.synthFast).toHaveBeenCalledWith({
316-
execCmd: ['node', 'xxxxx.integ-test1.js'],
317-
output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot',
316+
execCmd: ['node', 'test/test-data/xxxxx.integ-test1.js'],
317+
output: 'test/test-data/cdk-integ.out.xxxxx.integ-test1.js.snapshot',
318318
env: expect.objectContaining({
319319
CDK_INTEG_ACCOUNT: '12345678',
320320
CDK_INTEG_REGION: 'test-region',
@@ -342,7 +342,7 @@ describe('IntegTest runIntegTests', () => {
342342
expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
343343
expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
344344
expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({
345-
app: 'node xxxxx.integ-test1.js',
345+
app: 'node test/test-data/xxxxx.integ-test1.js',
346346
requireApproval: 'never',
347347
pathMetadata: false,
348348
assetMetadata: false,
@@ -355,10 +355,10 @@ describe('IntegTest runIntegTests', () => {
355355
profile: 'test-profile',
356356
lookups: false,
357357
stacks: ['stack1'],
358-
output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot',
358+
output: 'test/test-data/cdk-integ.out.xxxxx.integ-test1.js.snapshot',
359359
});
360360
expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({
361-
app: 'node xxxxx.integ-test1.js',
361+
app: 'node test/test-data/xxxxx.integ-test1.js',
362362
pathMetadata: false,
363363
assetMetadata: false,
364364
versionReporting: false,
@@ -370,7 +370,7 @@ describe('IntegTest runIntegTests', () => {
370370
profile: 'test-profile',
371371
force: true,
372372
all: true,
373-
output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot',
373+
output: 'test/test-data/cdk-integ.out.xxxxx.integ-test1.js.snapshot',
374374
});
375375
});
376376

@@ -625,13 +625,13 @@ describe('IntegTest runIntegTests', () => {
625625
expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
626626
expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
627627
expect(cdkMock.mocks.deploy).toHaveBeenCalledWith(expect.objectContaining({
628-
app: 'node --no-warnings xxxxx.test-with-snapshot.js',
628+
app: 'node --no-warnings test/test-data/xxxxx.test-with-snapshot.js',
629629
}));
630630
expect(cdkMock.mocks.synthFast).toHaveBeenCalledWith(expect.objectContaining({
631-
execCmd: ['node', '--no-warnings', 'xxxxx.test-with-snapshot.js'],
631+
execCmd: ['node', '--no-warnings', 'test/test-data/xxxxx.test-with-snapshot.js'],
632632
}));
633633
expect(cdkMock.mocks.destroy).toHaveBeenCalledWith(expect.objectContaining({
634-
app: 'node --no-warnings xxxxx.test-with-snapshot.js',
634+
app: 'node --no-warnings test/test-data/xxxxx.test-with-snapshot.js',
635635
}));
636636
});
637637
});
@@ -655,7 +655,7 @@ describe('IntegTest watchIntegTest', () => {
655655

656656
// THEN
657657
expect(cdkMock.mocks.watch).toHaveBeenCalledWith(expect.objectContaining({
658-
app: 'node --no-warnings xxxxx.test-with-snapshot.js',
658+
app: 'node --no-warnings test/test-data/xxxxx.test-with-snapshot.js',
659659
hotswap: HotswapMode.FALL_BACK,
660660
watch: true,
661661
traceLogs: false,
@@ -683,7 +683,7 @@ describe('IntegTest watchIntegTest', () => {
683683

684684
// THEN
685685
expect(cdkMock.mocks.watch).toHaveBeenCalledWith(expect.objectContaining({
686-
app: 'node --no-warnings xxxxx.test-with-snapshot.js',
686+
app: 'node --no-warnings test/test-data/xxxxx.test-with-snapshot.js',
687687
hotswap: HotswapMode.FALL_BACK,
688688
watch: true,
689689
traceLogs: true,

‎packages/@aws-cdk/integ-runner/test/runner/snapshot-test-runner.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,13 +219,13 @@ describe('IntegTest runSnapshotTests', () => {
219219
}));
220220
expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
221221
expect(cdkMock.mocks.synthFast).toHaveBeenCalledWith({
222-
execCmd: ['node', 'xxxxx.integ-test2.js'],
222+
execCmd: ['node', 'test/test-data/xxxxx.integ-test2.js'],
223223
env: expect.objectContaining({
224224
CDK_INTEG_ACCOUNT: '12345678',
225225
CDK_INTEG_REGION: 'test-region',
226226
}),
227227
context: expect.any(Object),
228-
output: '../../does/not/exist',
228+
output: 'does/not/exist',
229229
});
230230
});
231231
});

‎packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ describe('test runner', () => {
102102

103103
expect(spawnSyncMock).toHaveBeenCalledWith(
104104
expect.stringMatching(/node/),
105-
['xxxxx.integ-test1.js'],
105+
['test/test-data/xxxxx.integ-test1.js'],
106106
expect.objectContaining({
107107
env: expect.objectContaining({
108108
CDK_INTEG_ACCOUNT: '12345678',

0 commit comments

Comments
 (0)