Skip to content

Commit 887161c

Browse files
tejaskashtejaskash
committed
fix: address review comments on Days 5-6
1. cleanupStaleCredentialProviders default restored to 30 min, now configurable via E2E_STALE_CRED_MAX_AGE_MS env var. 5 min was too aggressive given container builds >5 min. 2. afterAll blocks that call teardownE2EProject now wrap in try/finally so testDir cleanup always runs even when teardown throws. Updated createE2ESuite + 5 standalone suites (ab-test-*, archive-lifecycle, config-bundle-eval-rec, evals-lifecycle). 3. vitest coverage include adds src/**/*.tsx so TUI React files are gated. Also exclude *.test.tsx from the coverage set. Adjusted src/cli/commands/ ratchet from 46% to 34% to reflect the new denominator (was 3424 lines, now 4553 with .tsx included). 4. check-coverage.mjs no longer silently skips changed files missing from coverage-final.json — counts them as 0% covered instead.
1 parent 2fffb14 commit 887161c

8 files changed

Lines changed: 48 additions & 24 deletions

‎e2e-tests/ab-test-config-bundle.test.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,13 @@ describe.sequential('e2e: config-bundle AB test lifecycle', () => {
5454
}, 300000);
5555

5656
afterAll(async () => {
57-
if (projectPath && hasAws) {
58-
await teardownE2EProject(projectPath, agentName, 'Bedrock');
57+
try {
58+
if (projectPath && hasAws) {
59+
await teardownE2EProject(projectPath, agentName, 'Bedrock');
60+
}
61+
} finally {
62+
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
5963
}
60-
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
6164
}, 600000);
6265

6366
const run = (args: string[]) => runAgentCoreCLI(args, projectPath);

‎e2e-tests/ab-test-target-based.test.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,13 @@ describe.sequential('e2e: target-based AB test lifecycle', () => {
5555
}, 300000);
5656

5757
afterAll(async () => {
58-
if (projectPath && hasAws) {
59-
await teardownE2EProject(projectPath, agentName, 'Bedrock');
58+
try {
59+
if (projectPath && hasAws) {
60+
await teardownE2EProject(projectPath, agentName, 'Bedrock');
61+
}
62+
} finally {
63+
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
6064
}
61-
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
6265
}, 600000);
6366

6467
const run = (args: string[]) => runAgentCoreCLI(args, projectPath);

‎e2e-tests/archive-lifecycle.test.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,13 @@ describe.sequential('e2e: archive command lifecycle', () => {
6767
}, 300000);
6868

6969
afterAll(async () => {
70-
if (projectPath && hasAws) {
71-
await teardownE2EProject(projectPath, agentName, 'Bedrock');
70+
try {
71+
if (projectPath && hasAws) {
72+
await teardownE2EProject(projectPath, agentName, 'Bedrock');
73+
}
74+
} finally {
75+
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
7276
}
73-
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
7477
}, 600000);
7578

7679
const run = (args: string[]) => runAgentCoreCLI(args, projectPath);

‎e2e-tests/config-bundle-eval-rec.test.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,13 @@ describe.sequential('e2e: config bundles, batch evaluation, and recommendations'
6464
}, 300000);
6565

6666
afterAll(async () => {
67-
if (projectPath && hasAws) {
68-
await teardownE2EProject(projectPath, agentName, 'Bedrock');
67+
try {
68+
if (projectPath && hasAws) {
69+
await teardownE2EProject(projectPath, agentName, 'Bedrock');
70+
}
71+
} finally {
72+
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
6973
}
70-
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
7174
}, 600000);
7275

7376
const run = (args: string[]) => runAgentCoreCLI(args, projectPath);

‎e2e-tests/e2e-helper.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,13 @@ export function createE2ESuite(cfg: E2EConfig) {
9696
}, 300000);
9797

9898
afterAll(async () => {
99-
if (projectPath && hasAws) {
100-
await teardownE2EProject(projectPath, agentName, cfg.modelProvider);
99+
try {
100+
if (projectPath && hasAws) {
101+
await teardownE2EProject(projectPath, agentName, cfg.modelProvider);
102+
}
103+
} finally {
104+
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
101105
}
102-
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
103106
}, 600000);
104107

105108
// Container builds go through CodeBuild which is slower and more prone to transient failures.
@@ -350,7 +353,9 @@ async function deleteCredentialProvider(client: BedrockAgentCoreControlClient, n
350353
* Runs in beforeAll to prevent accumulation from previous runs that
351354
* crashed or timed out before their afterAll teardown could execute.
352355
*/
353-
export async function cleanupStaleCredentialProviders(maxAgeMs: number = 5 * 60 * 1000): Promise<void> {
356+
export async function cleanupStaleCredentialProviders(
357+
maxAgeMs: number = parseInt(process.env.E2E_STALE_CRED_MAX_AGE_MS ?? '', 10) || 30 * 60 * 1000
358+
): Promise<void> {
354359
const region = process.env.AWS_REGION ?? 'us-east-1';
355360
const client = new BedrockAgentCoreControlClient({ region });
356361
const cutoff = new Date(Date.now() - maxAgeMs);

‎e2e-tests/evals-lifecycle.test.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,13 @@ describe.sequential('e2e: evaluations lifecycle', () => {
5353
}, 300000);
5454

5555
afterAll(async () => {
56-
if (projectPath && hasAws) {
57-
await teardownE2EProject(projectPath, agentName, 'Bedrock');
56+
try {
57+
if (projectPath && hasAws) {
58+
await teardownE2EProject(projectPath, agentName, 'Bedrock');
59+
}
60+
} finally {
61+
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
5862
}
59-
if (testDir) await rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 1000 });
6063
}, 600000);
6164

6265
const run = (args: string[]) => runAgentCoreCLI(args, projectPath);

‎scripts/check-coverage.mjs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const DIRECTORY_THRESHOLDS = [
3535
{ prefix: 'src/cli/aws/', threshold: 40, target: 60 },
3636
{ prefix: 'src/cli/tui/hooks/', threshold: 14, target: 55 },
3737
{ prefix: 'src/cli/tui/components/', threshold: 68, target: 75 },
38-
{ prefix: 'src/cli/commands/', threshold: 46, target: 60 },
38+
{ prefix: 'src/cli/commands/', threshold: 34, target: 60 },
3939
];
4040

4141
// ---------------------------------------------------------------------------
@@ -139,12 +139,15 @@ if (baseSha && headSha) {
139139
}
140140

141141
const coveredSet = coveredLinesByFile[file];
142-
if (!coveredSet) continue;
143142

144143
let fileCovered = 0;
145-
for (const line of lines) {
146-
if (coveredSet.has(line)) fileCovered++;
144+
if (coveredSet) {
145+
for (const line of lines) {
146+
if (coveredSet.has(line)) fileCovered++;
147+
}
147148
}
149+
// If coveredSet is undefined, the file was never loaded during tests —
150+
// count all changed lines as uncovered rather than skipping silently.
148151

149152
prLinesTotal += lines.length;
150153
prLinesCovered += fileCovered;

‎vitest.config.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,10 @@ export default defineConfig({
8686
reporter: ['text', 'text-summary', 'json', 'json-summary', 'html', 'lcov'],
8787
reportsDirectory: './coverage',
8888
reportOnFailure: true,
89-
include: ['src/**/*.ts'],
89+
include: ['src/**/*.ts', 'src/**/*.tsx'],
9090
exclude: [
9191
'src/**/*.test.ts',
92+
'src/**/*.test.tsx',
9293
'src/**/__tests__/**',
9394
'src/assets/**',
9495
'src/test-utils/**',

0 commit comments

Comments
 (0)