diff --git a/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts b/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts index 5035f9e07..a51fe5718 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts @@ -11,7 +11,7 @@ import { IntegRunner } from './runner-base'; import * as logger from '../logger'; import { chunks, exec, promiseWithResolvers } from '../utils'; import type { DestructiveChange, AssertionResults, AssertionResult } from '../workers/common'; -import { DiagnosticReason, formatAssertionResults } from '../workers/common'; +import { DiagnosticReason, formatAssertionResults, formatError } from '../workers/common'; export interface CommonOptions { /** @@ -114,15 +114,19 @@ export class IntegTestRunner extends IntegRunner { * all branches and we then search for one that starts with `HEAD branch: ` */ private checkoutSnapshot(): void { - const cwd = this.directory; + // We use the directory that contains the snapshot to run git commands in + // We don't change the cwd for executing git, but instead use the -C flag + // @see https://git-scm.com/docs/git#Documentation/git.txt--Cltpathgt + // This way we are guaranteed to operate under the correct git repo, even + // when executing integ-runner from outside the repo under test. + const gitCwd = path.dirname(this.snapshotDir); + const git = ['git', '-C', gitCwd]; // https://git-scm.com/docs/git-merge-base let baseBranch: string | undefined = undefined; // try to find the base branch that the working branch was created from try { - const origin: string = exec(['git', 'remote', 'show', 'origin'], { - cwd, - }); + const origin: string = exec([...git, 'remote', 'show', 'origin']); const originLines = origin.split('\n'); for (const line of originLines) { if (line.trim().startsWith('HEAD branch: ')) { @@ -141,22 +145,18 @@ export class IntegTestRunner extends IntegRunner { // if we found the base branch then get the merge-base (most recent common commit) // and checkout the snapshot using that commit if (baseBranch) { - const relativeSnapshotDir = path.relative(this.directory, this.snapshotDir); + const relativeSnapshotDir = path.relative(gitCwd, this.snapshotDir); try { - const base = exec(['git', 'merge-base', 'HEAD', baseBranch], { - cwd, - }); - exec(['git', 'checkout', base, '--', relativeSnapshotDir], { - cwd, - }); + const base = exec([...git, 'merge-base', 'HEAD', baseBranch]); + exec([...git, 'checkout', base, '--', relativeSnapshotDir]); } catch (e) { logger.warning('%s\n%s', `Could not checkout snapshot directory '${this.snapshotDir}'. Please verify the following command completes correctly:`, - `git checkout $(git merge-base HEAD ${baseBranch}) -- ${relativeSnapshotDir}`, + `git -C ${gitCwd} checkout $(git -C ${gitCwd} merge-base HEAD ${baseBranch}) -- ${relativeSnapshotDir}`, '', ); - logger.warning('error: %s', e); + logger.warning('error: %s', formatError(e)); } } } diff --git a/packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts b/packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts index 0f26b2cf0..9828ab70a 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts @@ -104,24 +104,19 @@ export class IntegTest { constructor(public readonly info: IntegTestInfo) { this.appCommand = info.appCommand ?? 'node {filePath}'; - this.absoluteFileName = path.resolve(info.fileName); - this.fileName = path.relative(process.cwd(), info.fileName); - const parsed = path.parse(this.fileName); + // for consistency, always run the CDK apps under test from the CWD + // this is especially important for languages that use the CWD to discover assets + // @see https://github.com/aws/aws-cdk-cli/issues/638 + this.directory = process.cwd(); + this.absoluteFileName = path.resolve(info.fileName); + this.fileName = path.relative(this.directory, info.fileName); this.discoveryRelativeFileName = path.relative(info.discoveryRoot, info.fileName); - // if `--watch` then we need the directory to be the cwd - this.directory = info.watch ? process.cwd() : parsed.dir; - - // if we are running in a package directory then just use the fileName - // as the testname, but if we are running in a parent directory with - // multiple packages then use the directory/filename as the testname - // - // Looks either like `integ.mytest` or `package/test/integ.mytest`. - const relDiscoveryRoot = path.relative(process.cwd(), info.discoveryRoot); - this.testName = this.directory === path.join(relDiscoveryRoot, 'test') || this.directory === path.join(relDiscoveryRoot) - ? parsed.name - : path.join(path.relative(this.info.discoveryRoot, parsed.dir), parsed.name); + // We treat the discovery root as the base for display names + // Looks either like `integ.mytest` or `package/test/integ.mytest` + const parsed = path.parse(this.fileName); + this.testName = path.join(path.relative(this.info.discoveryRoot, parsed.dir), parsed.name); this.normalizedTestName = parsed.name; this.snapshotDir = path.join(parsed.dir, `${parsed.base}.snapshot`); this.temporaryOutputDir = path.join(parsed.dir, `${CDK_OUTDIR_PREFIX}.${parsed.base}.snapshot`); diff --git a/packages/@aws-cdk/integ-runner/test/helpers.ts b/packages/@aws-cdk/integ-runner/test/helpers.ts index dda1059d5..e026abd21 100644 --- a/packages/@aws-cdk/integ-runner/test/helpers.ts +++ b/packages/@aws-cdk/integ-runner/test/helpers.ts @@ -78,6 +78,8 @@ export class MockCdkProvider { diagnostics: Diagnostic[]; destructiveChanges: DestructiveChange[]; }> { + const actualSnapshotLocation = actualSnapshot ? 'test/test-data/' + actualSnapshot : undefined; + // WHEN const integTest = new IntegSnapshotRunner({ cdk: this.cdk, @@ -85,7 +87,7 @@ export class MockCdkProvider { fileName: 'test/test-data/' + integTestFile, discoveryRoot: 'test/test-data', }), - integOutDir: actualSnapshot ? 'test/test-data/' + actualSnapshot : undefined, + integOutDir: actualSnapshotLocation, }); const results = await integTest.testSnapshot(); @@ -98,8 +100,8 @@ export class MockCdkProvider { CDK_INTEG_REGION: 'test-region', }), context: expect.any(Object), - execCmd: ['node', integTestFile], - output: actualSnapshot ?? `cdk-integ.out.${integTestFile}.snapshot`, + execCmd: ['node', 'test/test-data/' + integTestFile], + output: actualSnapshotLocation ?? `test/test-data/cdk-integ.out.${integTestFile}.snapshot`, }); return results; diff --git a/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts b/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts index cb0f879f8..3868741d2 100644 --- a/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts +++ b/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts @@ -61,7 +61,7 @@ describe('IntegTest runIntegTests', () => { expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2); expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({ - app: 'xxxxx.test-with-snapshot.js.snapshot', + app: 'test/test-data/xxxxx.test-with-snapshot.js.snapshot', requireApproval: 'never', pathMetadata: false, assetMetadata: false, @@ -76,11 +76,11 @@ describe('IntegTest runIntegTests', () => { stacks: ['test-stack'], }); expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({ - app: 'node xxxxx.test-with-snapshot.js', + app: 'node test/test-data/xxxxx.test-with-snapshot.js', requireApproval: 'never', pathMetadata: false, assetMetadata: false, - output: 'cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot', + output: 'test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot', profile: undefined, context: expect.not.objectContaining({ 'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({ @@ -92,7 +92,7 @@ describe('IntegTest runIntegTests', () => { stacks: ['test-stack', 'new-test-stack'], }); expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({ - app: 'node xxxxx.test-with-snapshot.js', + app: 'node test/test-data/xxxxx.test-with-snapshot.js', pathMetadata: false, assetMetadata: false, context: expect.not.objectContaining({ @@ -104,7 +104,7 @@ describe('IntegTest runIntegTests', () => { profile: undefined, force: true, all: true, - output: 'cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot', + output: 'test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot', }); }); @@ -126,7 +126,7 @@ describe('IntegTest runIntegTests', () => { expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2); expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({ - app: 'node xxxxx.integ-test1.js', + app: 'node test/test-data/xxxxx.integ-test1.js', requireApproval: 'never', pathMetadata: false, assetMetadata: false, @@ -137,17 +137,17 @@ describe('IntegTest runIntegTests', () => { }), lookups: false, stacks: ['stack1'], - output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot', + output: 'test/test-data/cdk-integ.out.xxxxx.integ-test1.js.snapshot', }); expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({ - app: 'node xxxxx.integ-test1.js', + app: 'node test/test-data/xxxxx.integ-test1.js', pathMetadata: false, assetMetadata: false, versionReporting: false, context: expect.any(Object), force: true, all: true, - output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot', + output: 'test/test-data/cdk-integ.out.xxxxx.integ-test1.js.snapshot', }); }); @@ -169,7 +169,7 @@ describe('IntegTest runIntegTests', () => { expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2); expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({ - app: 'node xxxxx.test-with-snapshot-assets-diff.js', + app: 'node test/test-data/xxxxx.test-with-snapshot-assets-diff.js', requireApproval: 'never', pathMetadata: false, assetMetadata: false, @@ -181,11 +181,11 @@ describe('IntegTest runIntegTests', () => { versionReporting: false, lookups: true, stacks: ['test-stack'], - output: 'cdk-integ.out.xxxxx.test-with-snapshot-assets-diff.js.snapshot', + output: 'test/test-data/cdk-integ.out.xxxxx.test-with-snapshot-assets-diff.js.snapshot', profile: undefined, }); expect(cdkMock.mocks.synthFast).toHaveBeenCalledWith({ - execCmd: ['node', 'xxxxx.test-with-snapshot-assets-diff.js'], + execCmd: ['node', 'test/test-data/xxxxx.test-with-snapshot-assets-diff.js'], env: expect.objectContaining({ CDK_INTEG_ACCOUNT: '12345678', CDK_INTEG_REGION: 'test-region', @@ -195,10 +195,10 @@ describe('IntegTest runIntegTests', () => { vpcId: 'vpc-60900905', }), }), - output: 'xxxxx.test-with-snapshot-assets-diff.js.snapshot', + output: 'test/test-data/xxxxx.test-with-snapshot-assets-diff.js.snapshot', }); expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({ - app: 'node xxxxx.test-with-snapshot-assets-diff.js', + app: 'node test/test-data/xxxxx.test-with-snapshot-assets-diff.js', pathMetadata: false, assetMetadata: false, context: expect.not.objectContaining({ @@ -209,7 +209,7 @@ describe('IntegTest runIntegTests', () => { versionReporting: false, force: true, all: true, - output: 'cdk-integ.out.xxxxx.test-with-snapshot-assets-diff.js.snapshot', + output: 'test/test-data/cdk-integ.out.xxxxx.test-with-snapshot-assets-diff.js.snapshot', }); }); @@ -231,7 +231,7 @@ describe('IntegTest runIntegTests', () => { expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2); expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(1, expect.objectContaining({ - app: 'xxxxx.test-with-snapshot.js.snapshot', + app: 'test/test-data/xxxxx.test-with-snapshot.js.snapshot', context: expect.any(Object), stacks: ['test-stack'], })); @@ -239,12 +239,12 @@ describe('IntegTest runIntegTests', () => { rollback: false, })); expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(3, expect.objectContaining({ - app: 'node xxxxx.test-with-snapshot.js', + app: 'node test/test-data/xxxxx.test-with-snapshot.js', stacks: ['Bundling/DefaultTest/DeployAssert'], rollback: false, })); expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({ - app: 'node xxxxx.test-with-snapshot.js', + app: 'node test/test-data/xxxxx.test-with-snapshot.js', pathMetadata: false, assetMetadata: false, context: expect.not.objectContaining({ @@ -255,7 +255,7 @@ describe('IntegTest runIntegTests', () => { versionReporting: false, force: true, all: true, - output: 'cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot', + output: 'test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot', }); }); @@ -313,8 +313,8 @@ describe('IntegTest runIntegTests', () => { // THEN expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.synthFast).toHaveBeenCalledWith({ - execCmd: ['node', 'xxxxx.integ-test1.js'], - output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot', + execCmd: ['node', 'test/test-data/xxxxx.integ-test1.js'], + output: 'test/test-data/cdk-integ.out.xxxxx.integ-test1.js.snapshot', env: expect.objectContaining({ CDK_INTEG_ACCOUNT: '12345678', CDK_INTEG_REGION: 'test-region', @@ -342,7 +342,7 @@ describe('IntegTest runIntegTests', () => { expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2); expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({ - app: 'node xxxxx.integ-test1.js', + app: 'node test/test-data/xxxxx.integ-test1.js', requireApproval: 'never', pathMetadata: false, assetMetadata: false, @@ -355,10 +355,10 @@ describe('IntegTest runIntegTests', () => { profile: 'test-profile', lookups: false, stacks: ['stack1'], - output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot', + output: 'test/test-data/cdk-integ.out.xxxxx.integ-test1.js.snapshot', }); expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({ - app: 'node xxxxx.integ-test1.js', + app: 'node test/test-data/xxxxx.integ-test1.js', pathMetadata: false, assetMetadata: false, versionReporting: false, @@ -370,7 +370,7 @@ describe('IntegTest runIntegTests', () => { profile: 'test-profile', force: true, all: true, - output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot', + output: 'test/test-data/cdk-integ.out.xxxxx.integ-test1.js.snapshot', }); }); @@ -442,13 +442,13 @@ describe('IntegTest runIntegTests', () => { // THEN expect(spawnSyncMock.mock.calls).toEqual(expect.arrayContaining([ expect.arrayContaining([ - 'git', ['remote', 'show', 'origin'], + 'git', ['-C', expect.any(String), 'remote', 'show', 'origin'], ]), expect.arrayContaining([ - 'git', ['merge-base', 'HEAD', 'main'], + 'git', ['-C', expect.any(String), 'merge-base', 'HEAD', 'main'], ]), expect.arrayContaining([ - 'git', ['checkout', 'abc', '--', 'xxxxx.test-with-snapshot.js.snapshot'], + 'git', ['-C', expect.any(String), 'checkout', 'abc', '--', 'xxxxx.test-with-snapshot.js.snapshot'], ]), ])); }); @@ -625,13 +625,13 @@ describe('IntegTest runIntegTests', () => { expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2); expect(cdkMock.mocks.deploy).toHaveBeenCalledWith(expect.objectContaining({ - app: 'node --no-warnings xxxxx.test-with-snapshot.js', + app: 'node --no-warnings test/test-data/xxxxx.test-with-snapshot.js', })); expect(cdkMock.mocks.synthFast).toHaveBeenCalledWith(expect.objectContaining({ - execCmd: ['node', '--no-warnings', 'xxxxx.test-with-snapshot.js'], + execCmd: ['node', '--no-warnings', 'test/test-data/xxxxx.test-with-snapshot.js'], })); expect(cdkMock.mocks.destroy).toHaveBeenCalledWith(expect.objectContaining({ - app: 'node --no-warnings xxxxx.test-with-snapshot.js', + app: 'node --no-warnings test/test-data/xxxxx.test-with-snapshot.js', })); }); }); @@ -655,7 +655,7 @@ describe('IntegTest watchIntegTest', () => { // THEN expect(cdkMock.mocks.watch).toHaveBeenCalledWith(expect.objectContaining({ - app: 'node --no-warnings xxxxx.test-with-snapshot.js', + app: 'node --no-warnings test/test-data/xxxxx.test-with-snapshot.js', hotswap: HotswapMode.FALL_BACK, watch: true, traceLogs: false, @@ -683,7 +683,7 @@ describe('IntegTest watchIntegTest', () => { // THEN expect(cdkMock.mocks.watch).toHaveBeenCalledWith(expect.objectContaining({ - app: 'node --no-warnings xxxxx.test-with-snapshot.js', + app: 'node --no-warnings test/test-data/xxxxx.test-with-snapshot.js', hotswap: HotswapMode.FALL_BACK, watch: true, traceLogs: true, diff --git a/packages/@aws-cdk/integ-runner/test/runner/snapshot-test-runner.test.ts b/packages/@aws-cdk/integ-runner/test/runner/snapshot-test-runner.test.ts index a32c1beaf..77decf086 100644 --- a/packages/@aws-cdk/integ-runner/test/runner/snapshot-test-runner.test.ts +++ b/packages/@aws-cdk/integ-runner/test/runner/snapshot-test-runner.test.ts @@ -219,13 +219,13 @@ describe('IntegTest runSnapshotTests', () => { })); expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.synthFast).toHaveBeenCalledWith({ - execCmd: ['node', 'xxxxx.integ-test2.js'], + execCmd: ['node', 'test/test-data/xxxxx.integ-test2.js'], env: expect.objectContaining({ CDK_INTEG_ACCOUNT: '12345678', CDK_INTEG_REGION: 'test-region', }), context: expect.any(Object), - output: '../../does/not/exist', + output: 'does/not/exist', }); }); }); diff --git a/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts b/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts index 977d29fbe..c2adec079 100644 --- a/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts +++ b/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts @@ -102,7 +102,7 @@ describe('test runner', () => { expect(spawnSyncMock).toHaveBeenCalledWith( expect.stringMatching(/node/), - ['xxxxx.integ-test1.js'], + ['test/test-data/xxxxx.integ-test1.js'], expect.objectContaining({ env: expect.objectContaining({ CDK_INTEG_ACCOUNT: '12345678', @@ -155,24 +155,15 @@ describe('test runner', () => { expect(spawnSyncMock.mock.calls).toEqual(expect.arrayContaining([ expect.arrayContaining([ expect.stringMatching(/git/), - ['remote', 'show', 'origin'], - expect.objectContaining({ - cwd: 'test/test-data', - }), + ['-C', 'test/test-data', 'remote', 'show', 'origin'], ]), expect.arrayContaining([ expect.stringMatching(/git/), - ['merge-base', 'HEAD', 'master'], - expect.objectContaining({ - cwd: 'test/test-data', - }), + ['-C', 'test/test-data', 'merge-base', 'HEAD', 'master'], ]), expect.arrayContaining([ expect.stringMatching(/git/), - ['checkout', 'abc', '--', 'xxxxx.test-with-snapshot.js.snapshot'], - expect.objectContaining({ - cwd: 'test/test-data', - }), + ['-C', 'test/test-data', 'checkout', 'abc', '--', 'xxxxx.test-with-snapshot.js.snapshot'], ]), ]));