Skip to content

Commit 51a1406

Browse files
authored
fix(integ-runner): error when running the update workflow with toolkit-lib engine (#717)
Fixes #715 The update workflow will deploy an existing snapshot from the main branch of the project before attempting to update the stack with incoming changes. When the toolkit-lib engine was used, the snapshot directory was incorrectly treated as an CDK app command and failing. The fix is to check if the app value is a directory and if so, just use it as a cloud assembly directory. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent f1a3a7d commit 51a1406

File tree

3 files changed

+106
-1
lines changed

3 files changed

+106
-1
lines changed

packages/@aws-cdk/integ-runner/lib/engines/toolkit-lib.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { DefaultCdkOptions, DestroyOptions } from '@aws-cdk/cloud-assembly-
44
import type { DeploymentMethod, ICloudAssemblySource, IIoHost, IoMessage, IoRequest, NonInteractiveIoHostProps, StackSelector } from '@aws-cdk/toolkit-lib';
55
import { ExpandStackSelection, MemoryContext, NonInteractiveIoHost, StackSelectionStrategy, Toolkit } from '@aws-cdk/toolkit-lib';
66
import * as chalk from 'chalk';
7+
import * as fs from 'fs-extra';
78

89
export interface ToolkitLibEngineOptions {
910
/**
@@ -201,6 +202,12 @@ export class ToolkitLibRunnerEngine implements ICdk {
201202
throw new Error('No app provided');
202203
}
203204

205+
// check if the app is a path to existing snapshot and then use it as an assembly directory
206+
const potentialCxPath = path.join(this.options.workingDirectory, options.app);
207+
if (fs.pathExistsSync(potentialCxPath) && fs.statSync(potentialCxPath).isDirectory()) {
208+
return this.toolkit.fromAssemblyDirectory(potentialCxPath);
209+
}
210+
204211
let outdir;
205212
if (options.output) {
206213
outdir = path.join(this.options.workingDirectory, options.output);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ export class IntegTestRunner extends IntegRunner {
454454
});
455455
}
456456
// if the update workflow is not disabled, first
457-
// perform a deployment with the exising snapshot
457+
// perform a deployment with the existing snapshot
458458
// then perform a deployment (which will be a stack update)
459459
// with the current integration test
460460
// We also only want to run the update workflow if there is an existing
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/* eslint-disable @typescript-eslint/unbound-method */
2+
import * as path from 'path';
3+
import { Toolkit } from '@aws-cdk/toolkit-lib';
4+
import * as fs from 'fs-extra';
5+
import { ToolkitLibRunnerEngine } from '../../lib/engines/toolkit-lib';
6+
7+
jest.mock('@aws-cdk/toolkit-lib');
8+
jest.mock('fs-extra');
9+
10+
const MockedToolkit = Toolkit as jest.MockedClass<typeof Toolkit>;
11+
const mockedFs = fs as jest.Mocked<typeof fs>;
12+
13+
describe('ToolkitLibRunnerEngine - Snapshot Path Handling', () => {
14+
let mockToolkit: jest.Mocked<Toolkit>;
15+
let engine: ToolkitLibRunnerEngine;
16+
17+
beforeEach(() => {
18+
jest.clearAllMocks();
19+
mockToolkit = {
20+
fromCdkApp: jest.fn(),
21+
fromAssemblyDirectory: jest.fn(),
22+
synth: jest.fn(),
23+
} as any;
24+
MockedToolkit.mockImplementation(() => mockToolkit);
25+
26+
engine = new ToolkitLibRunnerEngine({
27+
workingDirectory: '/test/dir',
28+
});
29+
});
30+
31+
it('should use fromAssemblyDirectory when app is a path to existing snapshot directory', async () => {
32+
const snapshotPath = 'test.snapshot';
33+
const fullSnapshotPath = path.join('/test/dir', snapshotPath);
34+
const mockCx = { produce: jest.fn() };
35+
const mockLock = { dispose: jest.fn() };
36+
37+
// Mock fs to indicate the snapshot directory exists
38+
mockedFs.pathExistsSync.mockReturnValue(true);
39+
mockedFs.statSync.mockReturnValue({ isDirectory: () => true } as any);
40+
41+
mockToolkit.fromAssemblyDirectory.mockResolvedValue(mockCx as any);
42+
mockToolkit.synth.mockResolvedValue(mockLock as any);
43+
44+
await engine.synth({
45+
app: snapshotPath,
46+
stacks: ['stack1'],
47+
});
48+
49+
expect(mockedFs.pathExistsSync).toHaveBeenCalledWith(fullSnapshotPath);
50+
expect(mockedFs.statSync).toHaveBeenCalledWith(fullSnapshotPath);
51+
expect(mockToolkit.fromAssemblyDirectory).toHaveBeenCalledWith(fullSnapshotPath);
52+
expect(mockToolkit.fromCdkApp).not.toHaveBeenCalled();
53+
});
54+
55+
it('should use fromCdkApp when app is not a path to existing directory', async () => {
56+
const appCommand = 'node bin/app.js';
57+
const mockCx = { produce: jest.fn() };
58+
const mockLock = { dispose: jest.fn() };
59+
60+
// Mock fs to indicate the path doesn't exist
61+
mockedFs.pathExistsSync.mockReturnValue(false);
62+
63+
mockToolkit.fromCdkApp.mockResolvedValue(mockCx as any);
64+
mockToolkit.synth.mockResolvedValue(mockLock as any);
65+
66+
await engine.synth({
67+
app: appCommand,
68+
stacks: ['stack1'],
69+
});
70+
71+
expect(mockToolkit.fromCdkApp).toHaveBeenCalledWith(appCommand, expect.any(Object));
72+
expect(mockToolkit.fromAssemblyDirectory).not.toHaveBeenCalled();
73+
});
74+
75+
it('should use fromCdkApp when app path exists but is not a directory', async () => {
76+
const appPath = 'app.js';
77+
const fullAppPath = path.join('/test/dir', appPath);
78+
const mockCx = { produce: jest.fn() };
79+
const mockLock = { dispose: jest.fn() };
80+
81+
// Mock fs to indicate the path exists but is not a directory
82+
mockedFs.pathExistsSync.mockReturnValue(true);
83+
mockedFs.statSync.mockReturnValue({ isDirectory: () => false } as any);
84+
85+
mockToolkit.fromCdkApp.mockResolvedValue(mockCx as any);
86+
mockToolkit.synth.mockResolvedValue(mockLock as any);
87+
88+
await engine.synth({
89+
app: appPath,
90+
stacks: ['stack1'],
91+
});
92+
93+
expect(mockedFs.pathExistsSync).toHaveBeenCalledWith(fullAppPath);
94+
expect(mockedFs.statSync).toHaveBeenCalledWith(fullAppPath);
95+
expect(mockToolkit.fromCdkApp).toHaveBeenCalledWith(appPath, expect.any(Object));
96+
expect(mockToolkit.fromAssemblyDirectory).not.toHaveBeenCalled();
97+
});
98+
});

0 commit comments

Comments
 (0)