Skip to content

Commit bf94b40

Browse files
aidandaly24aidandaly24
committed
chore: remove dead code and redundant tests from dockerfile PR
- Remove unused ADVANCED_GROUP_LABELS constant (dead code) - Remove unnecessary export on DOCKERFILE_NAME_REGEX - Fix stale `steps` dependency in useGenerateWizard setAdvanced callback - Trim computeByoSteps.test.ts to dockerfile-only tests (remove 11 tests for pre-existing behavior unchanged by this PR) - Remove redundant "uses default Dockerfile" tests that duplicate existing coverage in preflight, config, and container packager test files - Consolidate shell metacharacter it.each from 5 cases to 1 representative Confidence: high Scope-risk: narrow
1 parent b814968 commit bf94b40

File tree

7 files changed

+44
-249
lines changed

7 files changed

+44
-249
lines changed

‎src/cli/operations/deploy/__tests__/preflight-container.test.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,4 @@ describe('validateContainerAgents', () => {
124124

125125
expect(() => validateContainerAgents(spec, CONFIG_ROOT)).toThrow(/Dockerfile\.gpu not found/);
126126
});
127-
128-
it('uses default Dockerfile when no custom dockerfile specified', () => {
129-
mockedExistsSync.mockReturnValue(true);
130-
131-
const spec = makeSpec([{ name: 'default-agent', build: 'Container', codeLocation: dir('agents/default') }]);
132-
133-
expect(() => validateContainerAgents(spec, CONFIG_ROOT)).not.toThrow();
134-
const calledPath = mockedExistsSync.mock.calls[0]?.[0] as string;
135-
expect(calledPath).toMatch(/\/Dockerfile$/);
136-
});
137127
});

‎src/cli/operations/dev/__tests__/config.test.ts‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -396,34 +396,6 @@ describe('getDevConfig', () => {
396396
expect(config).not.toBeNull();
397397
expect(config?.dockerfile).toBe('Dockerfile.gpu');
398398
});
399-
400-
it('returns undefined dockerfile when agent has no custom dockerfile', () => {
401-
const project: AgentCoreProjectSpec = {
402-
name: 'TestProject',
403-
version: 1,
404-
managedBy: 'CDK' as const,
405-
runtimes: [
406-
{
407-
name: 'ContainerAgent',
408-
build: 'Container',
409-
runtimeVersion: 'PYTHON_3_12',
410-
entrypoint: filePath('main.py'),
411-
codeLocation: dirPath('./agents/container'),
412-
protocol: 'HTTP',
413-
},
414-
],
415-
memories: [],
416-
credentials: [],
417-
evaluators: [],
418-
onlineEvalConfigs: [],
419-
agentCoreGateways: [],
420-
policyEngines: [],
421-
};
422-
423-
const config = getDevConfig(workingDir, project, '/test/project/agentcore');
424-
expect(config).not.toBeNull();
425-
expect(config?.dockerfile).toBeUndefined();
426-
});
427399
});
428400

429401
describe('getAgentPort', () => {

‎src/cli/tui/screens/agent/__tests__/computeByoSteps.test.ts‎

Lines changed: 38 additions & 170 deletions
Original file line numberDiff line numberDiff line change
@@ -14,181 +14,49 @@ function makeInput(overrides: Partial<ComputeByoStepsInput> = {}): ComputeByoSte
1414
};
1515
}
1616

17-
describe('computeByoSteps', () => {
18-
describe('base steps', () => {
19-
it('Bedrock provider excludes apiKey', () => {
20-
const steps = computeByoSteps(makeInput({ modelProvider: 'Bedrock' }));
21-
expect(steps).not.toContain('apiKey');
22-
expect(steps).toEqual(['codeLocation', 'buildType', 'modelProvider', 'advanced', 'confirm']);
23-
});
24-
25-
it('non-Bedrock provider includes apiKey', () => {
26-
const steps = computeByoSteps(makeInput({ modelProvider: 'OpenAI' }));
27-
expect(steps).toContain('apiKey');
28-
expect(steps).toEqual(['codeLocation', 'buildType', 'modelProvider', 'apiKey', 'advanced', 'confirm']);
29-
});
30-
});
31-
32-
describe('dockerfile advanced setting', () => {
33-
it('Container build with dockerfile selected includes dockerfile step', () => {
34-
const steps = computeByoSteps(
35-
makeInput({
36-
buildType: 'Container',
37-
advancedSettings: new Set<AdvancedSettingId>(['dockerfile']),
38-
})
39-
);
40-
expect(steps).toContain('dockerfile');
41-
const advIdx = steps.indexOf('advanced');
42-
expect(steps[advIdx + 1]).toBe('dockerfile');
43-
});
44-
45-
it('CodeZip build with dockerfile selected does NOT include dockerfile step', () => {
46-
const steps = computeByoSteps(
47-
makeInput({
48-
buildType: 'CodeZip',
49-
advancedSettings: new Set<AdvancedSettingId>(['dockerfile']),
50-
})
51-
);
52-
expect(steps).not.toContain('dockerfile');
53-
});
54-
55-
it('dockerfile-only selection on Container has steps: advanced, dockerfile, confirm', () => {
56-
const steps = computeByoSteps(
57-
makeInput({
58-
buildType: 'Container',
59-
advancedSettings: new Set<AdvancedSettingId>(['dockerfile']),
60-
})
61-
);
62-
const advIdx = steps.indexOf('advanced');
63-
expect(steps.slice(advIdx)).toEqual(['advanced', 'dockerfile', 'confirm']);
64-
});
65-
});
66-
67-
describe('network advanced setting', () => {
68-
it('network selected adds networkMode', () => {
69-
const steps = computeByoSteps(
70-
makeInput({
71-
advancedSettings: new Set<AdvancedSettingId>(['network']),
72-
})
73-
);
74-
expect(steps).toContain('networkMode');
75-
expect(steps).not.toContain('subnets');
76-
});
77-
78-
it('network + VPC adds subnets and securityGroups', () => {
79-
const steps = computeByoSteps(
80-
makeInput({
81-
networkMode: 'VPC',
82-
advancedSettings: new Set<AdvancedSettingId>(['network']),
83-
})
84-
);
85-
const advIdx = steps.indexOf('advanced');
86-
expect(steps.slice(advIdx)).toEqual(['advanced', 'networkMode', 'subnets', 'securityGroups', 'confirm']);
87-
});
17+
describe('computeByoSteps - dockerfile', () => {
18+
it('Container build with dockerfile selected includes dockerfile step', () => {
19+
const steps = computeByoSteps(
20+
makeInput({
21+
buildType: 'Container',
22+
advancedSettings: new Set<AdvancedSettingId>(['dockerfile']),
23+
})
24+
);
25+
expect(steps).toContain('dockerfile');
26+
const advIdx = steps.indexOf('advanced');
27+
expect(steps[advIdx + 1]).toBe('dockerfile');
8828
});
8929

90-
describe('partial advanced selections', () => {
91-
it('headers-only adds requestHeaderAllowlist', () => {
92-
const steps = computeByoSteps(
93-
makeInput({
94-
advancedSettings: new Set<AdvancedSettingId>(['headers']),
95-
})
96-
);
97-
const advIdx = steps.indexOf('advanced');
98-
expect(steps.slice(advIdx)).toEqual(['advanced', 'requestHeaderAllowlist', 'confirm']);
99-
});
100-
101-
it('auth-only adds authorizerType', () => {
102-
const steps = computeByoSteps(
103-
makeInput({
104-
advancedSettings: new Set<AdvancedSettingId>(['auth']),
105-
})
106-
);
107-
const advIdx = steps.indexOf('advanced');
108-
expect(steps.slice(advIdx)).toEqual(['advanced', 'authorizerType', 'confirm']);
109-
});
110-
111-
it('lifecycle-only adds idleTimeout and maxLifetime', () => {
112-
const steps = computeByoSteps(
113-
makeInput({
114-
advancedSettings: new Set<AdvancedSettingId>(['lifecycle']),
115-
})
116-
);
117-
const advIdx = steps.indexOf('advanced');
118-
expect(steps.slice(advIdx)).toEqual(['advanced', 'idleTimeout', 'maxLifetime', 'confirm']);
119-
});
120-
121-
it('dockerfile + lifecycle on Container includes both groups', () => {
122-
const steps = computeByoSteps(
123-
makeInput({
124-
buildType: 'Container',
125-
advancedSettings: new Set<AdvancedSettingId>(['dockerfile', 'lifecycle']),
126-
})
127-
);
128-
const advIdx = steps.indexOf('advanced');
129-
expect(steps.slice(advIdx)).toEqual(['advanced', 'dockerfile', 'idleTimeout', 'maxLifetime', 'confirm']);
130-
expect(steps).not.toContain('networkMode');
131-
});
30+
it('CodeZip build with dockerfile selected does NOT include dockerfile step', () => {
31+
const steps = computeByoSteps(
32+
makeInput({
33+
buildType: 'CodeZip',
34+
advancedSettings: new Set<AdvancedSettingId>(['dockerfile']),
35+
})
36+
);
37+
expect(steps).not.toContain('dockerfile');
13238
});
13339

134-
describe('full selection', () => {
135-
it('all settings on Container + VPC produces complete sub-step list', () => {
136-
const steps = computeByoSteps(
137-
makeInput({
138-
buildType: 'Container',
139-
networkMode: 'VPC',
140-
advancedSettings: new Set<AdvancedSettingId>(['dockerfile', 'network', 'headers', 'auth', 'lifecycle']),
141-
})
142-
);
143-
const advIdx = steps.indexOf('advanced');
144-
expect(steps.slice(advIdx)).toEqual([
145-
'advanced',
146-
'dockerfile',
147-
'networkMode',
148-
'subnets',
149-
'securityGroups',
150-
'requestHeaderAllowlist',
151-
'authorizerType',
152-
'idleTimeout',
153-
'maxLifetime',
154-
'confirm',
155-
]);
156-
});
40+
it('dockerfile-only selection on Container has steps: advanced, dockerfile, confirm', () => {
41+
const steps = computeByoSteps(
42+
makeInput({
43+
buildType: 'Container',
44+
advancedSettings: new Set<AdvancedSettingId>(['dockerfile']),
45+
})
46+
);
47+
const advIdx = steps.indexOf('advanced');
48+
expect(steps.slice(advIdx)).toEqual(['advanced', 'dockerfile', 'confirm']);
15749
});
15850

159-
describe('empty selection', () => {
160-
it('no advanced settings means no sub-steps', () => {
161-
const steps = computeByoSteps(
162-
makeInput({
163-
advancedSettings: new Set<AdvancedSettingId>(),
164-
})
165-
);
166-
const advIdx = steps.indexOf('advanced');
167-
expect(steps.slice(advIdx)).toEqual(['advanced', 'confirm']);
168-
});
169-
});
170-
171-
describe('CUSTOM_JWT injects jwtConfig', () => {
172-
it('CUSTOM_JWT with auth selected adds jwtConfig after authorizerType', () => {
173-
const steps = computeByoSteps(
174-
makeInput({
175-
authorizerType: 'CUSTOM_JWT',
176-
advancedSettings: new Set<AdvancedSettingId>(['auth']),
177-
})
178-
);
179-
const authIdx = steps.indexOf('authorizerType');
180-
expect(steps[authIdx + 1]).toBe('jwtConfig');
181-
});
182-
183-
it('CUSTOM_JWT without auth selected does not add jwtConfig', () => {
184-
const steps = computeByoSteps(
185-
makeInput({
186-
authorizerType: 'CUSTOM_JWT',
187-
advancedSettings: new Set<AdvancedSettingId>(),
188-
})
189-
);
190-
expect(steps).not.toContain('jwtConfig');
191-
expect(steps).not.toContain('authorizerType');
192-
});
51+
it('dockerfile + lifecycle on Container includes both groups', () => {
52+
const steps = computeByoSteps(
53+
makeInput({
54+
buildType: 'Container',
55+
advancedSettings: new Set<AdvancedSettingId>(['dockerfile', 'lifecycle']),
56+
})
57+
);
58+
const advIdx = steps.indexOf('advanced');
59+
expect(steps.slice(advIdx)).toEqual(['advanced', 'dockerfile', 'idleTimeout', 'maxLifetime', 'confirm']);
60+
expect(steps).not.toContain('networkMode');
19361
});
19462
});

‎src/cli/tui/screens/generate/types.ts‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ export const ADVANCED_SETTING_OPTIONS = [
164164
] as const;
165165

166166
/** Dockerfile filename regex — must match the Zod schema in agent-env.ts */
167-
export const DOCKERFILE_NAME_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/;
167+
const DOCKERFILE_NAME_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/;
168168

169169
/**
170170
* Validate a Dockerfile input value from the TUI.
@@ -187,15 +187,6 @@ export function validateDockerfileInput(value: string): true | string {
187187
return true;
188188
}
189189

190-
/** Group labels for the advanced sub-step indicator */
191-
export const ADVANCED_GROUP_LABELS: Record<AdvancedSettingId, string> = {
192-
dockerfile: 'Container',
193-
network: 'Network',
194-
headers: 'Headers',
195-
auth: 'Auth',
196-
lifecycle: 'Lifecycle',
197-
};
198-
199190
export const MEMORY_OPTIONS = [
200191
{ id: 'none', title: 'None', description: 'No memory' },
201192
{ id: 'shortTerm', title: 'Short-term memory', description: 'Context within a session' },

‎src/cli/tui/screens/generate/useGenerateWizard.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ export function useGenerateWizard(options?: UseGenerateWizardOptions) {
252252
}, 0);
253253
}
254254
},
255-
[config.buildType, steps]
255+
[config.buildType]
256256
);
257257

258258
const setNetworkMode = useCallback(

‎src/lib/packaging/__tests__/container.test.ts‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -202,28 +202,6 @@ describe('ContainerPackager', () => {
202202
expect(buildArgs[fIdx + 1]).toBe('/resolved/src/Dockerfile.gpu');
203203
});
204204

205-
it('uses default Dockerfile when no custom dockerfile specified', async () => {
206-
mockResolveCodeLocation.mockReturnValue('/resolved/src');
207-
mockExistsSync.mockReturnValue(true);
208-
mockSpawnSync.mockImplementation((cmd: string, args: string[]) => {
209-
if (cmd === 'which' && args[0] === 'docker') return { status: 0 };
210-
if (cmd === 'docker' && args[0] === '--version') return { status: 0 };
211-
if (cmd === 'docker' && args[0] === 'build') return { status: 0 };
212-
if (cmd === 'docker' && args[0] === 'image') return { status: 0, stdout: Buffer.from('1000') };
213-
return { status: 1 };
214-
});
215-
216-
await packager.pack(baseSpec as any);
217-
218-
const buildCall = mockSpawnSync.mock.calls.find(
219-
(c: unknown[]) => c[0] === 'docker' && (c[1] as string[])[0] === 'build'
220-
);
221-
expect(buildCall).toBeDefined();
222-
const buildArgs = buildCall![1] as string[];
223-
const fIdx = buildArgs.indexOf('-f');
224-
expect(buildArgs[fIdx + 1]).toBe('/resolved/src/Dockerfile');
225-
});
226-
227205
it('rejects when custom dockerfile not found', async () => {
228206
mockResolveCodeLocation.mockReturnValue('/resolved/src');
229207
mockExistsSync.mockReturnValue(false);

‎src/schema/schemas/__tests__/agent-env.test.ts‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -463,14 +463,10 @@ describe('AgentEnvSpecSchema - dockerfile', () => {
463463
expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: '' }).success).toBe(false);
464464
});
465465

466-
it.each([
467-
'Dockerfile;rm -rf /',
468-
'Dockerfile$(whoami)',
469-
'Dockerfile`id`',
470-
'Dockerfile | cat /etc/passwd',
471-
'Dockerfile&& echo pwned',
472-
])('rejects shell metacharacters in dockerfile "%s"', name => {
473-
expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: name }).success).toBe(false);
466+
it('rejects shell metacharacters in dockerfile', () => {
467+
expect(AgentEnvSpecSchema.safeParse({ ...validContainerAgent, dockerfile: 'Dockerfile;rm -rf /' }).success).toBe(
468+
false
469+
);
474470
});
475471

476472
it('rejects dockerfile exceeding 255 characters', () => {

0 commit comments

Comments
 (0)