Skip to content

ci: cut full e2e time in half via vitest sharding - #1016

Merged
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard
Apr 30, 2026
Merged

ci: cut full e2e time in half via vitest sharding#1016
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard

Conversation

@Hweinstock

@HweinstockHweinstock commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Description

The full E2E suite takes 19-25 min because all 17 test files run on a single runner per CDK source. Each test deploys its own CloudFormation stack, and measured timings show parallel deploys have zero degradation (77s stays 77s with concurrent stacks).

This adds 6-way vitest sharding via the GitHub Actions matrix, going from 2 to 12 parallel runners. Also isolates import test resources per run to prevent conflicts when npm and main matrix entries hit the same AWS account concurrently.

Measured results:

ShardsWall-clockSlowest shard
1 (baseline)19-25 min
4~15 min890s
6~11.5 min691s
8~12.0 min717s

6 shards appears to be the sweet spot. Adding more shards creates diminishing returns since the main bottleneck is the deploy (274s) and container build (204s). This is roughly what the e2e tests take on PRs when running minimal files so we're likely close to how fast we can get it.

Inspired by #989.

Docs: https://vitest.dev/guide/improving-performance.html#sharding

Import test resource isolation

The import-resources.test.ts setup creates AWS resources (runtime, memory, evaluator) via Python scripts, then saves their ARNs to bugbash-resources.json. The cleanup script deletes those resources and also calls cleanup_s3_code_objects() which previously deleted all objects from the shared S3 bucket bugbash-agentcore-code-{account}-{region}.

The npm and main matrix entries are separate runners hitting the same AWS account simultaneously. Both run import-resources.test.ts. When one job's cleanup finishes first, it nuked all S3 objects — including the code.zip the other job uploaded and was still using, causing downstream resource creation failures.

The fix: each run now gets a unique RESOURCE_SUFFIX (the test's timestamp-based suffix). This scopes:

  • The resources file: bugbash-resources-{suffix}.json instead of bugbash-resources.json
  • The S3 prefix: bugbash-{suffix}/code.zip instead of bugbash/code.zip
  • The S3 cleanup: only deletes objects under its own prefix

Related Issue

N/A — CI performance improvement.

Documentation PR

N/A

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe): CI performance — shard E2E tests for faster wall-clock time

Testing

Tried different shard sizes, runs are here: https://github.com/aws/agentcore-cli/actions/runs/25165632571, https://github.com/aws/agentcore-cli/actions/runs/25166733599

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed — looks good to merge.

Verified:

  • vitest run --project e2e --shard=X/4 is a supported combination in vitest 4.x
  • 16 e2e test files distribute across 4 shards (~4 files each); imbalance is acceptable given fail-fast: false and 60-minute timeout
  • Parallel test concerns look fine: cleanupStaleCredentialProviders() uses a 30-min cutoff so it won't touch resources from concurrently running shards; agent names include framework/provider prefixes so they won't collide across files
  • No production code touched; failure mode is just a CI run that surfaces the issue, easy to revert

One thing worth noting (not blocking): your local measurement validated 2 concurrent deploys, but this moves to 8 concurrent (4 shards × 2 cdk-sources). If you see intermittent CloudFormation/AgentCore control-plane throttling in the first few scheduled runs, that'd be the likely cause — but worth finding out empirically rather than preemptively capping.

@github-actionsgithub-actionsBot removed agentcore-harness-reviewing AgentCore Harness review in progress size/xs PR size: XS labels Apr 29, 2026
@github-actionsgithub-actionsBot added the size/s PR size: S label Apr 29, 2026
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
- Add 6-way vitest sharding to the cdk-source matrix (2 → 12 parallel runners)
- Isolate import test resources per run via RESOURCE_SUFFIX to prevent concurrent conflicts
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

/strands review

@HweinstockHweinstock changed the title ci: cut e2e time in half via vitest shardingci: cut full e2e time in half via vitest shardingApr 30, 2026
@Hweinstock
Hweinstock marked this pull request as ready for review April 30, 2026 13:36
@Hweinstock
Hweinstock requested a review from a teamApril 30, 2026 13:36
tejaskash
tejaskash previously approved these changes Apr 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Code Review Summary

I've reviewed PR #1016, which adds 6-way vitest sharding to reduce E2E test time from 19-25 min to ~11.5 min. The approach is sound and well-measured, but there is one blocking issue that must be fixed before merge.


🔴 Blocking Issues

Missing import in cleanup_resources.py

File:e2e-tests/fixtures/import/cleanup_resources.py
Lines: 15, 25

The script uses RESOURCE_SUFFIX on line 25 but doesn't import it from common.py on line 15:

# Line 15 - current importsfromcommonimportREGION, RESOURCES_FILE, get_control_client, get_account_id# Line 25 - uses RESOURCE_SUFFIX (not imported!)prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""

Impact: This will cause a NameError: name 'RESOURCE_SUFFIX' is not defined when cleanup runs after a test, potentially leaving resources in AWS.

Fix: Add RESOURCE_SUFFIX to the import statement:

fromcommonimportREGION, RESOURCES_FILE, RESOURCE_SUFFIX, get_control_client, get_account_id

💡 Suggestions (non-blocking)

1. Resource suffix collision risk

Files:e2e-tests/import-resources.test.ts:37, e2e-tests/import-gateway.test.ts:34

Both tests generate the suffix using Date.now():

constsuffix=Date.now().toString().slice(-8);

In a sharded run, if two shards start within the same millisecond and run the same test, they could collide on resource names. While unlikely, using a more unique identifier would be more robust:

constsuffix=`${Date.now()}-${randomUUID().slice(0,8)}`;

Or include the shard number if available in the environment.


2. S3 cleanup safety concern

File:e2e-tests/fixtures/import/cleanup_resources.py:25

When RESOURCE_SUFFIX is empty or unset, the prefix becomes "", which means list_objects_v2 will return all objects in the bucket:

prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""list_args= {"Bucket": bucket_name}
ifprefix: # Empty string is falsy, so this branch is skippedlist_args["Prefix"] =prefixresp=s3.list_objects_v2(**list_args) # Lists ALL objects

This could accidentally delete objects from other concurrent runs if the env var is missing. Consider:

  1. Making RESOURCE_SUFFIX required (fail fast if unset)
  2. Or use a fallback like "default" instead of empty string

3. Documentation: Why 6 shards?

File:.github/workflows/e2e-tests-full.yml:30

The PR description explains the measured results for 4, 6, and 8 shards, and that 6 is optimal. Consider adding a brief comment in the workflow file explaining why 6 was chosen:

matrix:
cdk-source: [npm, main]# 6 shards provides optimal balance (~11.5 min wall-clock)# 8 shards showed no improvement due to memory deploy (274s) and container build (204s) bottlenecksshard: ['1/6', '2/6', '3/6', '4/6', '5/6', '6/6']

❓ Questions

Concurrency increase: 2 → 12 parallel deploys

File:.github/workflows/e2e-tests-full.yml:28-30

The PR moves from 2 concurrent runners (2 cdk-source entries) to 12 (2 × 6 shards). The description mentions testing 2 concurrent deploys locally but not 12.

Question: Have you verified that the AWS account's CloudFormation/AgentCore control plane can handle 12 concurrent stacks without throttling? The bot comment mentioned this could be "worth finding out empirically," but it would be good to confirm if you've already done a test run with the full matrix.


✅ Positive Aspects

  • Well-measured performance improvement: Clear data showing 19-25 min → 11.5 min
  • Proper resource isolation: Using RESOURCE_SUFFIX to scope S3 objects and resources files per run prevents conflicts between npm and main matrix entries
  • No production code changes: Only CI and test infrastructure, easy to revert if issues arise
  • Comprehensive testing: The PR description shows you tested 4, 6, and 8 shards to find the optimal configuration

Next Steps

  1. Fix the blocking import issue in cleanup_resources.py
  2. Consider the suggestions above (especially the S3 cleanup safety concern)
  3. Confirm the 12 concurrent deploys won't throttle in CI

Overall, this is a great performance improvement with a solid approach. Once the import bug is fixed, this should be good to merge! 🚀

@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

Addressed comments and re ran https://github.com/aws/agentcore-cli/actions/runs/25175265881.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sPR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Hweinstock@agentcore-cli-automation@tejaskash
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
ci: cut full e2e time in half via vitest sharding by Hweinstock · Pull Request #1016 · aws/agentcore-cli · GitHub
Skip to content

ci: cut full e2e time in half via vitest sharding - #1016

Merged
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard
Apr 30, 2026
Merged

ci: cut full e2e time in half via vitest sharding#1016
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard

Conversation

@Hweinstock

@HweinstockHweinstock commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Description

The full E2E suite takes 19-25 min because all 17 test files run on a single runner per CDK source. Each test deploys its own CloudFormation stack, and measured timings show parallel deploys have zero degradation (77s stays 77s with concurrent stacks).

This adds 6-way vitest sharding via the GitHub Actions matrix, going from 2 to 12 parallel runners. Also isolates import test resources per run to prevent conflicts when npm and main matrix entries hit the same AWS account concurrently.

Measured results:

ShardsWall-clockSlowest shard
1 (baseline)19-25 min
4~15 min890s
6~11.5 min691s
8~12.0 min717s

6 shards appears to be the sweet spot. Adding more shards creates diminishing returns since the main bottleneck is the deploy (274s) and container build (204s). This is roughly what the e2e tests take on PRs when running minimal files so we're likely close to how fast we can get it.

Inspired by #989.

Docs: https://vitest.dev/guide/improving-performance.html#sharding

Import test resource isolation

The import-resources.test.ts setup creates AWS resources (runtime, memory, evaluator) via Python scripts, then saves their ARNs to bugbash-resources.json. The cleanup script deletes those resources and also calls cleanup_s3_code_objects() which previously deleted all objects from the shared S3 bucket bugbash-agentcore-code-{account}-{region}.

The npm and main matrix entries are separate runners hitting the same AWS account simultaneously. Both run import-resources.test.ts. When one job's cleanup finishes first, it nuked all S3 objects — including the code.zip the other job uploaded and was still using, causing downstream resource creation failures.

The fix: each run now gets a unique RESOURCE_SUFFIX (the test's timestamp-based suffix). This scopes:

  • The resources file: bugbash-resources-{suffix}.json instead of bugbash-resources.json
  • The S3 prefix: bugbash-{suffix}/code.zip instead of bugbash/code.zip
  • The S3 cleanup: only deletes objects under its own prefix

Related Issue

N/A — CI performance improvement.

Documentation PR

N/A

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe): CI performance — shard E2E tests for faster wall-clock time

Testing

Tried different shard sizes, runs are here: https://github.com/aws/agentcore-cli/actions/runs/25165632571, https://github.com/aws/agentcore-cli/actions/runs/25166733599

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed — looks good to merge.

Verified:

  • vitest run --project e2e --shard=X/4 is a supported combination in vitest 4.x
  • 16 e2e test files distribute across 4 shards (~4 files each); imbalance is acceptable given fail-fast: false and 60-minute timeout
  • Parallel test concerns look fine: cleanupStaleCredentialProviders() uses a 30-min cutoff so it won't touch resources from concurrently running shards; agent names include framework/provider prefixes so they won't collide across files
  • No production code touched; failure mode is just a CI run that surfaces the issue, easy to revert

One thing worth noting (not blocking): your local measurement validated 2 concurrent deploys, but this moves to 8 concurrent (4 shards × 2 cdk-sources). If you see intermittent CloudFormation/AgentCore control-plane throttling in the first few scheduled runs, that'd be the likely cause — but worth finding out empirically rather than preemptively capping.

@github-actionsgithub-actionsBot removed agentcore-harness-reviewing AgentCore Harness review in progress size/xs PR size: XS labels Apr 29, 2026
@github-actionsgithub-actionsBot added the size/s PR size: S label Apr 29, 2026
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
- Add 6-way vitest sharding to the cdk-source matrix (2 → 12 parallel runners)
- Isolate import test resources per run via RESOURCE_SUFFIX to prevent concurrent conflicts
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

/strands review

@HweinstockHweinstock changed the title ci: cut e2e time in half via vitest shardingci: cut full e2e time in half via vitest shardingApr 30, 2026
@Hweinstock
Hweinstock marked this pull request as ready for review April 30, 2026 13:36
@Hweinstock
Hweinstock requested a review from a teamApril 30, 2026 13:36
tejaskash
tejaskash previously approved these changes Apr 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Code Review Summary

I've reviewed PR #1016, which adds 6-way vitest sharding to reduce E2E test time from 19-25 min to ~11.5 min. The approach is sound and well-measured, but there is one blocking issue that must be fixed before merge.


🔴 Blocking Issues

Missing import in cleanup_resources.py

File:e2e-tests/fixtures/import/cleanup_resources.py
Lines: 15, 25

The script uses RESOURCE_SUFFIX on line 25 but doesn't import it from common.py on line 15:

# Line 15 - current importsfromcommonimportREGION, RESOURCES_FILE, get_control_client, get_account_id# Line 25 - uses RESOURCE_SUFFIX (not imported!)prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""

Impact: This will cause a NameError: name 'RESOURCE_SUFFIX' is not defined when cleanup runs after a test, potentially leaving resources in AWS.

Fix: Add RESOURCE_SUFFIX to the import statement:

fromcommonimportREGION, RESOURCES_FILE, RESOURCE_SUFFIX, get_control_client, get_account_id

💡 Suggestions (non-blocking)

1. Resource suffix collision risk

Files:e2e-tests/import-resources.test.ts:37, e2e-tests/import-gateway.test.ts:34

Both tests generate the suffix using Date.now():

constsuffix=Date.now().toString().slice(-8);

In a sharded run, if two shards start within the same millisecond and run the same test, they could collide on resource names. While unlikely, using a more unique identifier would be more robust:

constsuffix=`${Date.now()}-${randomUUID().slice(0,8)}`;

Or include the shard number if available in the environment.


2. S3 cleanup safety concern

File:e2e-tests/fixtures/import/cleanup_resources.py:25

When RESOURCE_SUFFIX is empty or unset, the prefix becomes "", which means list_objects_v2 will return all objects in the bucket:

prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""list_args= {"Bucket": bucket_name}
ifprefix: # Empty string is falsy, so this branch is skippedlist_args["Prefix"] =prefixresp=s3.list_objects_v2(**list_args) # Lists ALL objects

This could accidentally delete objects from other concurrent runs if the env var is missing. Consider:

  1. Making RESOURCE_SUFFIX required (fail fast if unset)
  2. Or use a fallback like "default" instead of empty string

3. Documentation: Why 6 shards?

File:.github/workflows/e2e-tests-full.yml:30

The PR description explains the measured results for 4, 6, and 8 shards, and that 6 is optimal. Consider adding a brief comment in the workflow file explaining why 6 was chosen:

matrix:
cdk-source: [npm, main]# 6 shards provides optimal balance (~11.5 min wall-clock)# 8 shards showed no improvement due to memory deploy (274s) and container build (204s) bottlenecksshard: ['1/6', '2/6', '3/6', '4/6', '5/6', '6/6']

❓ Questions

Concurrency increase: 2 → 12 parallel deploys

File:.github/workflows/e2e-tests-full.yml:28-30

The PR moves from 2 concurrent runners (2 cdk-source entries) to 12 (2 × 6 shards). The description mentions testing 2 concurrent deploys locally but not 12.

Question: Have you verified that the AWS account's CloudFormation/AgentCore control plane can handle 12 concurrent stacks without throttling? The bot comment mentioned this could be "worth finding out empirically," but it would be good to confirm if you've already done a test run with the full matrix.


✅ Positive Aspects

  • Well-measured performance improvement: Clear data showing 19-25 min → 11.5 min
  • Proper resource isolation: Using RESOURCE_SUFFIX to scope S3 objects and resources files per run prevents conflicts between npm and main matrix entries
  • No production code changes: Only CI and test infrastructure, easy to revert if issues arise
  • Comprehensive testing: The PR description shows you tested 4, 6, and 8 shards to find the optimal configuration

Next Steps

  1. Fix the blocking import issue in cleanup_resources.py
  2. Consider the suggestions above (especially the S3 cleanup safety concern)
  3. Confirm the 12 concurrent deploys won't throttle in CI

Overall, this is a great performance improvement with a solid approach. Once the import bug is fixed, this should be good to merge! 🚀

@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

Addressed comments and re ran https://github.com/aws/agentcore-cli/actions/runs/25175265881.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sPR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Hweinstock@agentcore-cli-automation@tejaskash
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci: cut full e2e time in half via vitest sharding by Hweinstock · Pull Request #1016 · aws/agentcore-cli · GitHub
Skip to content

ci: cut full e2e time in half via vitest sharding - #1016

Merged
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard
Apr 30, 2026
Merged

ci: cut full e2e time in half via vitest sharding#1016
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard

Conversation

@Hweinstock

@HweinstockHweinstock commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Description

The full E2E suite takes 19-25 min because all 17 test files run on a single runner per CDK source. Each test deploys its own CloudFormation stack, and measured timings show parallel deploys have zero degradation (77s stays 77s with concurrent stacks).

This adds 6-way vitest sharding via the GitHub Actions matrix, going from 2 to 12 parallel runners. Also isolates import test resources per run to prevent conflicts when npm and main matrix entries hit the same AWS account concurrently.

Measured results:

ShardsWall-clockSlowest shard
1 (baseline)19-25 min
4~15 min890s
6~11.5 min691s
8~12.0 min717s

6 shards appears to be the sweet spot. Adding more shards creates diminishing returns since the main bottleneck is the deploy (274s) and container build (204s). This is roughly what the e2e tests take on PRs when running minimal files so we're likely close to how fast we can get it.

Inspired by #989.

Docs: https://vitest.dev/guide/improving-performance.html#sharding

Import test resource isolation

The import-resources.test.ts setup creates AWS resources (runtime, memory, evaluator) via Python scripts, then saves their ARNs to bugbash-resources.json. The cleanup script deletes those resources and also calls cleanup_s3_code_objects() which previously deleted all objects from the shared S3 bucket bugbash-agentcore-code-{account}-{region}.

The npm and main matrix entries are separate runners hitting the same AWS account simultaneously. Both run import-resources.test.ts. When one job's cleanup finishes first, it nuked all S3 objects — including the code.zip the other job uploaded and was still using, causing downstream resource creation failures.

The fix: each run now gets a unique RESOURCE_SUFFIX (the test's timestamp-based suffix). This scopes:

  • The resources file: bugbash-resources-{suffix}.json instead of bugbash-resources.json
  • The S3 prefix: bugbash-{suffix}/code.zip instead of bugbash/code.zip
  • The S3 cleanup: only deletes objects under its own prefix

Related Issue

N/A — CI performance improvement.

Documentation PR

N/A

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe): CI performance — shard E2E tests for faster wall-clock time

Testing

Tried different shard sizes, runs are here: https://github.com/aws/agentcore-cli/actions/runs/25165632571, https://github.com/aws/agentcore-cli/actions/runs/25166733599

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed — looks good to merge.

Verified:

  • vitest run --project e2e --shard=X/4 is a supported combination in vitest 4.x
  • 16 e2e test files distribute across 4 shards (~4 files each); imbalance is acceptable given fail-fast: false and 60-minute timeout
  • Parallel test concerns look fine: cleanupStaleCredentialProviders() uses a 30-min cutoff so it won't touch resources from concurrently running shards; agent names include framework/provider prefixes so they won't collide across files
  • No production code touched; failure mode is just a CI run that surfaces the issue, easy to revert

One thing worth noting (not blocking): your local measurement validated 2 concurrent deploys, but this moves to 8 concurrent (4 shards × 2 cdk-sources). If you see intermittent CloudFormation/AgentCore control-plane throttling in the first few scheduled runs, that'd be the likely cause — but worth finding out empirically rather than preemptively capping.

@github-actionsgithub-actionsBot removed agentcore-harness-reviewing AgentCore Harness review in progress size/xs PR size: XS labels Apr 29, 2026
@github-actionsgithub-actionsBot added the size/s PR size: S label Apr 29, 2026
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
- Add 6-way vitest sharding to the cdk-source matrix (2 → 12 parallel runners)
- Isolate import test resources per run via RESOURCE_SUFFIX to prevent concurrent conflicts
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

/strands review

@HweinstockHweinstock changed the title ci: cut e2e time in half via vitest shardingci: cut full e2e time in half via vitest shardingApr 30, 2026
@Hweinstock
Hweinstock marked this pull request as ready for review April 30, 2026 13:36
@Hweinstock
Hweinstock requested a review from a teamApril 30, 2026 13:36
tejaskash
tejaskash previously approved these changes Apr 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Code Review Summary

I've reviewed PR #1016, which adds 6-way vitest sharding to reduce E2E test time from 19-25 min to ~11.5 min. The approach is sound and well-measured, but there is one blocking issue that must be fixed before merge.


🔴 Blocking Issues

Missing import in cleanup_resources.py

File:e2e-tests/fixtures/import/cleanup_resources.py
Lines: 15, 25

The script uses RESOURCE_SUFFIX on line 25 but doesn't import it from common.py on line 15:

# Line 15 - current importsfromcommonimportREGION, RESOURCES_FILE, get_control_client, get_account_id# Line 25 - uses RESOURCE_SUFFIX (not imported!)prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""

Impact: This will cause a NameError: name 'RESOURCE_SUFFIX' is not defined when cleanup runs after a test, potentially leaving resources in AWS.

Fix: Add RESOURCE_SUFFIX to the import statement:

fromcommonimportREGION, RESOURCES_FILE, RESOURCE_SUFFIX, get_control_client, get_account_id

💡 Suggestions (non-blocking)

1. Resource suffix collision risk

Files:e2e-tests/import-resources.test.ts:37, e2e-tests/import-gateway.test.ts:34

Both tests generate the suffix using Date.now():

constsuffix=Date.now().toString().slice(-8);

In a sharded run, if two shards start within the same millisecond and run the same test, they could collide on resource names. While unlikely, using a more unique identifier would be more robust:

constsuffix=`${Date.now()}-${randomUUID().slice(0,8)}`;

Or include the shard number if available in the environment.


2. S3 cleanup safety concern

File:e2e-tests/fixtures/import/cleanup_resources.py:25

When RESOURCE_SUFFIX is empty or unset, the prefix becomes "", which means list_objects_v2 will return all objects in the bucket:

prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""list_args= {"Bucket": bucket_name}
ifprefix: # Empty string is falsy, so this branch is skippedlist_args["Prefix"] =prefixresp=s3.list_objects_v2(**list_args) # Lists ALL objects

This could accidentally delete objects from other concurrent runs if the env var is missing. Consider:

  1. Making RESOURCE_SUFFIX required (fail fast if unset)
  2. Or use a fallback like "default" instead of empty string

3. Documentation: Why 6 shards?

File:.github/workflows/e2e-tests-full.yml:30

The PR description explains the measured results for 4, 6, and 8 shards, and that 6 is optimal. Consider adding a brief comment in the workflow file explaining why 6 was chosen:

matrix:
cdk-source: [npm, main]# 6 shards provides optimal balance (~11.5 min wall-clock)# 8 shards showed no improvement due to memory deploy (274s) and container build (204s) bottlenecksshard: ['1/6', '2/6', '3/6', '4/6', '5/6', '6/6']

❓ Questions

Concurrency increase: 2 → 12 parallel deploys

File:.github/workflows/e2e-tests-full.yml:28-30

The PR moves from 2 concurrent runners (2 cdk-source entries) to 12 (2 × 6 shards). The description mentions testing 2 concurrent deploys locally but not 12.

Question: Have you verified that the AWS account's CloudFormation/AgentCore control plane can handle 12 concurrent stacks without throttling? The bot comment mentioned this could be "worth finding out empirically," but it would be good to confirm if you've already done a test run with the full matrix.


✅ Positive Aspects

  • Well-measured performance improvement: Clear data showing 19-25 min → 11.5 min
  • Proper resource isolation: Using RESOURCE_SUFFIX to scope S3 objects and resources files per run prevents conflicts between npm and main matrix entries
  • No production code changes: Only CI and test infrastructure, easy to revert if issues arise
  • Comprehensive testing: The PR description shows you tested 4, 6, and 8 shards to find the optimal configuration

Next Steps

  1. Fix the blocking import issue in cleanup_resources.py
  2. Consider the suggestions above (especially the S3 cleanup safety concern)
  3. Confirm the 12 concurrent deploys won't throttle in CI

Overall, this is a great performance improvement with a solid approach. Once the import bug is fixed, this should be good to merge! 🚀

@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

Addressed comments and re ran https://github.com/aws/agentcore-cli/actions/runs/25175265881.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sPR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Hweinstock@agentcore-cli-automation@tejaskash
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci: cut full e2e time in half via vitest sharding by Hweinstock · Pull Request #1016 · aws/agentcore-cli · GitHub
Skip to content

ci: cut full e2e time in half via vitest sharding - #1016

Merged
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard
Apr 30, 2026
Merged

ci: cut full e2e time in half via vitest sharding#1016
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard

Conversation

@Hweinstock

@HweinstockHweinstock commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Description

The full E2E suite takes 19-25 min because all 17 test files run on a single runner per CDK source. Each test deploys its own CloudFormation stack, and measured timings show parallel deploys have zero degradation (77s stays 77s with concurrent stacks).

This adds 6-way vitest sharding via the GitHub Actions matrix, going from 2 to 12 parallel runners. Also isolates import test resources per run to prevent conflicts when npm and main matrix entries hit the same AWS account concurrently.

Measured results:

ShardsWall-clockSlowest shard
1 (baseline)19-25 min
4~15 min890s
6~11.5 min691s
8~12.0 min717s

6 shards appears to be the sweet spot. Adding more shards creates diminishing returns since the main bottleneck is the deploy (274s) and container build (204s). This is roughly what the e2e tests take on PRs when running minimal files so we're likely close to how fast we can get it.

Inspired by #989.

Docs: https://vitest.dev/guide/improving-performance.html#sharding

Import test resource isolation

The import-resources.test.ts setup creates AWS resources (runtime, memory, evaluator) via Python scripts, then saves their ARNs to bugbash-resources.json. The cleanup script deletes those resources and also calls cleanup_s3_code_objects() which previously deleted all objects from the shared S3 bucket bugbash-agentcore-code-{account}-{region}.

The npm and main matrix entries are separate runners hitting the same AWS account simultaneously. Both run import-resources.test.ts. When one job's cleanup finishes first, it nuked all S3 objects — including the code.zip the other job uploaded and was still using, causing downstream resource creation failures.

The fix: each run now gets a unique RESOURCE_SUFFIX (the test's timestamp-based suffix). This scopes:

  • The resources file: bugbash-resources-{suffix}.json instead of bugbash-resources.json
  • The S3 prefix: bugbash-{suffix}/code.zip instead of bugbash/code.zip
  • The S3 cleanup: only deletes objects under its own prefix

Related Issue

N/A — CI performance improvement.

Documentation PR

N/A

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe): CI performance — shard E2E tests for faster wall-clock time

Testing

Tried different shard sizes, runs are here: https://github.com/aws/agentcore-cli/actions/runs/25165632571, https://github.com/aws/agentcore-cli/actions/runs/25166733599

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed — looks good to merge.

Verified:

  • vitest run --project e2e --shard=X/4 is a supported combination in vitest 4.x
  • 16 e2e test files distribute across 4 shards (~4 files each); imbalance is acceptable given fail-fast: false and 60-minute timeout
  • Parallel test concerns look fine: cleanupStaleCredentialProviders() uses a 30-min cutoff so it won't touch resources from concurrently running shards; agent names include framework/provider prefixes so they won't collide across files
  • No production code touched; failure mode is just a CI run that surfaces the issue, easy to revert

One thing worth noting (not blocking): your local measurement validated 2 concurrent deploys, but this moves to 8 concurrent (4 shards × 2 cdk-sources). If you see intermittent CloudFormation/AgentCore control-plane throttling in the first few scheduled runs, that'd be the likely cause — but worth finding out empirically rather than preemptively capping.

@github-actionsgithub-actionsBot removed agentcore-harness-reviewing AgentCore Harness review in progress size/xs PR size: XS labels Apr 29, 2026
@github-actionsgithub-actionsBot added the size/s PR size: S label Apr 29, 2026
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
- Add 6-way vitest sharding to the cdk-source matrix (2 → 12 parallel runners)
- Isolate import test resources per run via RESOURCE_SUFFIX to prevent concurrent conflicts
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

/strands review

@HweinstockHweinstock changed the title ci: cut e2e time in half via vitest shardingci: cut full e2e time in half via vitest shardingApr 30, 2026
@Hweinstock
Hweinstock marked this pull request as ready for review April 30, 2026 13:36
@Hweinstock
Hweinstock requested a review from a teamApril 30, 2026 13:36
tejaskash
tejaskash previously approved these changes Apr 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Code Review Summary

I've reviewed PR #1016, which adds 6-way vitest sharding to reduce E2E test time from 19-25 min to ~11.5 min. The approach is sound and well-measured, but there is one blocking issue that must be fixed before merge.


🔴 Blocking Issues

Missing import in cleanup_resources.py

File:e2e-tests/fixtures/import/cleanup_resources.py
Lines: 15, 25

The script uses RESOURCE_SUFFIX on line 25 but doesn't import it from common.py on line 15:

# Line 15 - current importsfromcommonimportREGION, RESOURCES_FILE, get_control_client, get_account_id# Line 25 - uses RESOURCE_SUFFIX (not imported!)prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""

Impact: This will cause a NameError: name 'RESOURCE_SUFFIX' is not defined when cleanup runs after a test, potentially leaving resources in AWS.

Fix: Add RESOURCE_SUFFIX to the import statement:

fromcommonimportREGION, RESOURCES_FILE, RESOURCE_SUFFIX, get_control_client, get_account_id

💡 Suggestions (non-blocking)

1. Resource suffix collision risk

Files:e2e-tests/import-resources.test.ts:37, e2e-tests/import-gateway.test.ts:34

Both tests generate the suffix using Date.now():

constsuffix=Date.now().toString().slice(-8);

In a sharded run, if two shards start within the same millisecond and run the same test, they could collide on resource names. While unlikely, using a more unique identifier would be more robust:

constsuffix=`${Date.now()}-${randomUUID().slice(0,8)}`;

Or include the shard number if available in the environment.


2. S3 cleanup safety concern

File:e2e-tests/fixtures/import/cleanup_resources.py:25

When RESOURCE_SUFFIX is empty or unset, the prefix becomes "", which means list_objects_v2 will return all objects in the bucket:

prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""list_args= {"Bucket": bucket_name}
ifprefix: # Empty string is falsy, so this branch is skippedlist_args["Prefix"] =prefixresp=s3.list_objects_v2(**list_args) # Lists ALL objects

This could accidentally delete objects from other concurrent runs if the env var is missing. Consider:

  1. Making RESOURCE_SUFFIX required (fail fast if unset)
  2. Or use a fallback like "default" instead of empty string

3. Documentation: Why 6 shards?

File:.github/workflows/e2e-tests-full.yml:30

The PR description explains the measured results for 4, 6, and 8 shards, and that 6 is optimal. Consider adding a brief comment in the workflow file explaining why 6 was chosen:

matrix:
cdk-source: [npm, main]# 6 shards provides optimal balance (~11.5 min wall-clock)# 8 shards showed no improvement due to memory deploy (274s) and container build (204s) bottlenecksshard: ['1/6', '2/6', '3/6', '4/6', '5/6', '6/6']

❓ Questions

Concurrency increase: 2 → 12 parallel deploys

File:.github/workflows/e2e-tests-full.yml:28-30

The PR moves from 2 concurrent runners (2 cdk-source entries) to 12 (2 × 6 shards). The description mentions testing 2 concurrent deploys locally but not 12.

Question: Have you verified that the AWS account's CloudFormation/AgentCore control plane can handle 12 concurrent stacks without throttling? The bot comment mentioned this could be "worth finding out empirically," but it would be good to confirm if you've already done a test run with the full matrix.


✅ Positive Aspects

  • Well-measured performance improvement: Clear data showing 19-25 min → 11.5 min
  • Proper resource isolation: Using RESOURCE_SUFFIX to scope S3 objects and resources files per run prevents conflicts between npm and main matrix entries
  • No production code changes: Only CI and test infrastructure, easy to revert if issues arise
  • Comprehensive testing: The PR description shows you tested 4, 6, and 8 shards to find the optimal configuration

Next Steps

  1. Fix the blocking import issue in cleanup_resources.py
  2. Consider the suggestions above (especially the S3 cleanup safety concern)
  3. Confirm the 12 concurrent deploys won't throttle in CI

Overall, this is a great performance improvement with a solid approach. Once the import bug is fixed, this should be good to merge! 🚀

@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

Addressed comments and re ran https://github.com/aws/agentcore-cli/actions/runs/25175265881.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sPR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Hweinstock@agentcore-cli-automation@tejaskash
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' ci: cut full e2e time in half via vitest sharding by Hweinstock · Pull Request #1016 · aws/agentcore-cli · GitHub
Skip to content

ci: cut full e2e time in half via vitest sharding - #1016

Merged
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard
Apr 30, 2026
Merged

ci: cut full e2e time in half via vitest sharding#1016
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard

Conversation

@Hweinstock

@HweinstockHweinstock commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Description

The full E2E suite takes 19-25 min because all 17 test files run on a single runner per CDK source. Each test deploys its own CloudFormation stack, and measured timings show parallel deploys have zero degradation (77s stays 77s with concurrent stacks).

This adds 6-way vitest sharding via the GitHub Actions matrix, going from 2 to 12 parallel runners. Also isolates import test resources per run to prevent conflicts when npm and main matrix entries hit the same AWS account concurrently.

Measured results:

ShardsWall-clockSlowest shard
1 (baseline)19-25 min
4~15 min890s
6~11.5 min691s
8~12.0 min717s

6 shards appears to be the sweet spot. Adding more shards creates diminishing returns since the main bottleneck is the deploy (274s) and container build (204s). This is roughly what the e2e tests take on PRs when running minimal files so we're likely close to how fast we can get it.

Inspired by #989.

Docs: https://vitest.dev/guide/improving-performance.html#sharding

Import test resource isolation

The import-resources.test.ts setup creates AWS resources (runtime, memory, evaluator) via Python scripts, then saves their ARNs to bugbash-resources.json. The cleanup script deletes those resources and also calls cleanup_s3_code_objects() which previously deleted all objects from the shared S3 bucket bugbash-agentcore-code-{account}-{region}.

The npm and main matrix entries are separate runners hitting the same AWS account simultaneously. Both run import-resources.test.ts. When one job's cleanup finishes first, it nuked all S3 objects — including the code.zip the other job uploaded and was still using, causing downstream resource creation failures.

The fix: each run now gets a unique RESOURCE_SUFFIX (the test's timestamp-based suffix). This scopes:

  • The resources file: bugbash-resources-{suffix}.json instead of bugbash-resources.json
  • The S3 prefix: bugbash-{suffix}/code.zip instead of bugbash/code.zip
  • The S3 cleanup: only deletes objects under its own prefix

Related Issue

N/A — CI performance improvement.

Documentation PR

N/A

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe): CI performance — shard E2E tests for faster wall-clock time

Testing

Tried different shard sizes, runs are here: https://github.com/aws/agentcore-cli/actions/runs/25165632571, https://github.com/aws/agentcore-cli/actions/runs/25166733599

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed — looks good to merge.

Verified:

  • vitest run --project e2e --shard=X/4 is a supported combination in vitest 4.x
  • 16 e2e test files distribute across 4 shards (~4 files each); imbalance is acceptable given fail-fast: false and 60-minute timeout
  • Parallel test concerns look fine: cleanupStaleCredentialProviders() uses a 30-min cutoff so it won't touch resources from concurrently running shards; agent names include framework/provider prefixes so they won't collide across files
  • No production code touched; failure mode is just a CI run that surfaces the issue, easy to revert

One thing worth noting (not blocking): your local measurement validated 2 concurrent deploys, but this moves to 8 concurrent (4 shards × 2 cdk-sources). If you see intermittent CloudFormation/AgentCore control-plane throttling in the first few scheduled runs, that'd be the likely cause — but worth finding out empirically rather than preemptively capping.

@github-actionsgithub-actionsBot removed agentcore-harness-reviewing AgentCore Harness review in progress size/xs PR size: XS labels Apr 29, 2026
@github-actionsgithub-actionsBot added the size/s PR size: S label Apr 29, 2026
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
- Add 6-way vitest sharding to the cdk-source matrix (2 → 12 parallel runners)
- Isolate import test resources per run via RESOURCE_SUFFIX to prevent concurrent conflicts
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

/strands review

@HweinstockHweinstock changed the title ci: cut e2e time in half via vitest shardingci: cut full e2e time in half via vitest shardingApr 30, 2026
@Hweinstock
Hweinstock marked this pull request as ready for review April 30, 2026 13:36
@Hweinstock
Hweinstock requested a review from a teamApril 30, 2026 13:36
tejaskash
tejaskash previously approved these changes Apr 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Code Review Summary

I've reviewed PR #1016, which adds 6-way vitest sharding to reduce E2E test time from 19-25 min to ~11.5 min. The approach is sound and well-measured, but there is one blocking issue that must be fixed before merge.


🔴 Blocking Issues

Missing import in cleanup_resources.py

File:e2e-tests/fixtures/import/cleanup_resources.py
Lines: 15, 25

The script uses RESOURCE_SUFFIX on line 25 but doesn't import it from common.py on line 15:

# Line 15 - current importsfromcommonimportREGION, RESOURCES_FILE, get_control_client, get_account_id# Line 25 - uses RESOURCE_SUFFIX (not imported!)prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""

Impact: This will cause a NameError: name 'RESOURCE_SUFFIX' is not defined when cleanup runs after a test, potentially leaving resources in AWS.

Fix: Add RESOURCE_SUFFIX to the import statement:

fromcommonimportREGION, RESOURCES_FILE, RESOURCE_SUFFIX, get_control_client, get_account_id

💡 Suggestions (non-blocking)

1. Resource suffix collision risk

Files:e2e-tests/import-resources.test.ts:37, e2e-tests/import-gateway.test.ts:34

Both tests generate the suffix using Date.now():

constsuffix=Date.now().toString().slice(-8);

In a sharded run, if two shards start within the same millisecond and run the same test, they could collide on resource names. While unlikely, using a more unique identifier would be more robust:

constsuffix=`${Date.now()}-${randomUUID().slice(0,8)}`;

Or include the shard number if available in the environment.


2. S3 cleanup safety concern

File:e2e-tests/fixtures/import/cleanup_resources.py:25

When RESOURCE_SUFFIX is empty or unset, the prefix becomes "", which means list_objects_v2 will return all objects in the bucket:

prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""list_args= {"Bucket": bucket_name}
ifprefix: # Empty string is falsy, so this branch is skippedlist_args["Prefix"] =prefixresp=s3.list_objects_v2(**list_args) # Lists ALL objects

This could accidentally delete objects from other concurrent runs if the env var is missing. Consider:

  1. Making RESOURCE_SUFFIX required (fail fast if unset)
  2. Or use a fallback like "default" instead of empty string

3. Documentation: Why 6 shards?

File:.github/workflows/e2e-tests-full.yml:30

The PR description explains the measured results for 4, 6, and 8 shards, and that 6 is optimal. Consider adding a brief comment in the workflow file explaining why 6 was chosen:

matrix:
cdk-source: [npm, main]# 6 shards provides optimal balance (~11.5 min wall-clock)# 8 shards showed no improvement due to memory deploy (274s) and container build (204s) bottlenecksshard: ['1/6', '2/6', '3/6', '4/6', '5/6', '6/6']

❓ Questions

Concurrency increase: 2 → 12 parallel deploys

File:.github/workflows/e2e-tests-full.yml:28-30

The PR moves from 2 concurrent runners (2 cdk-source entries) to 12 (2 × 6 shards). The description mentions testing 2 concurrent deploys locally but not 12.

Question: Have you verified that the AWS account's CloudFormation/AgentCore control plane can handle 12 concurrent stacks without throttling? The bot comment mentioned this could be "worth finding out empirically," but it would be good to confirm if you've already done a test run with the full matrix.


✅ Positive Aspects

  • Well-measured performance improvement: Clear data showing 19-25 min → 11.5 min
  • Proper resource isolation: Using RESOURCE_SUFFIX to scope S3 objects and resources files per run prevents conflicts between npm and main matrix entries
  • No production code changes: Only CI and test infrastructure, easy to revert if issues arise
  • Comprehensive testing: The PR description shows you tested 4, 6, and 8 shards to find the optimal configuration

Next Steps

  1. Fix the blocking import issue in cleanup_resources.py
  2. Consider the suggestions above (especially the S3 cleanup safety concern)
  3. Confirm the 12 concurrent deploys won't throttle in CI

Overall, this is a great performance improvement with a solid approach. Once the import bug is fixed, this should be good to merge! 🚀

@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

Addressed comments and re ran https://github.com/aws/agentcore-cli/actions/runs/25175265881.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sPR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Hweinstock@agentcore-cli-automation@tejaskash
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci: cut full e2e time in half via vitest sharding by Hweinstock · Pull Request #1016 · aws/agentcore-cli · GitHub
Skip to content

ci: cut full e2e time in half via vitest sharding - #1016

Merged
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard
Apr 30, 2026
Merged

ci: cut full e2e time in half via vitest sharding#1016
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard

Conversation

@Hweinstock

@HweinstockHweinstock commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Description

The full E2E suite takes 19-25 min because all 17 test files run on a single runner per CDK source. Each test deploys its own CloudFormation stack, and measured timings show parallel deploys have zero degradation (77s stays 77s with concurrent stacks).

This adds 6-way vitest sharding via the GitHub Actions matrix, going from 2 to 12 parallel runners. Also isolates import test resources per run to prevent conflicts when npm and main matrix entries hit the same AWS account concurrently.

Measured results:

ShardsWall-clockSlowest shard
1 (baseline)19-25 min
4~15 min890s
6~11.5 min691s
8~12.0 min717s

6 shards appears to be the sweet spot. Adding more shards creates diminishing returns since the main bottleneck is the deploy (274s) and container build (204s). This is roughly what the e2e tests take on PRs when running minimal files so we're likely close to how fast we can get it.

Inspired by #989.

Docs: https://vitest.dev/guide/improving-performance.html#sharding

Import test resource isolation

The import-resources.test.ts setup creates AWS resources (runtime, memory, evaluator) via Python scripts, then saves their ARNs to bugbash-resources.json. The cleanup script deletes those resources and also calls cleanup_s3_code_objects() which previously deleted all objects from the shared S3 bucket bugbash-agentcore-code-{account}-{region}.

The npm and main matrix entries are separate runners hitting the same AWS account simultaneously. Both run import-resources.test.ts. When one job's cleanup finishes first, it nuked all S3 objects — including the code.zip the other job uploaded and was still using, causing downstream resource creation failures.

The fix: each run now gets a unique RESOURCE_SUFFIX (the test's timestamp-based suffix). This scopes:

  • The resources file: bugbash-resources-{suffix}.json instead of bugbash-resources.json
  • The S3 prefix: bugbash-{suffix}/code.zip instead of bugbash/code.zip
  • The S3 cleanup: only deletes objects under its own prefix

Related Issue

N/A — CI performance improvement.

Documentation PR

N/A

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe): CI performance — shard E2E tests for faster wall-clock time

Testing

Tried different shard sizes, runs are here: https://github.com/aws/agentcore-cli/actions/runs/25165632571, https://github.com/aws/agentcore-cli/actions/runs/25166733599

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed — looks good to merge.

Verified:

  • vitest run --project e2e --shard=X/4 is a supported combination in vitest 4.x
  • 16 e2e test files distribute across 4 shards (~4 files each); imbalance is acceptable given fail-fast: false and 60-minute timeout
  • Parallel test concerns look fine: cleanupStaleCredentialProviders() uses a 30-min cutoff so it won't touch resources from concurrently running shards; agent names include framework/provider prefixes so they won't collide across files
  • No production code touched; failure mode is just a CI run that surfaces the issue, easy to revert

One thing worth noting (not blocking): your local measurement validated 2 concurrent deploys, but this moves to 8 concurrent (4 shards × 2 cdk-sources). If you see intermittent CloudFormation/AgentCore control-plane throttling in the first few scheduled runs, that'd be the likely cause — but worth finding out empirically rather than preemptively capping.

@github-actionsgithub-actionsBot removed agentcore-harness-reviewing AgentCore Harness review in progress size/xs PR size: XS labels Apr 29, 2026
@github-actionsgithub-actionsBot added the size/s PR size: S label Apr 29, 2026
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
- Add 6-way vitest sharding to the cdk-source matrix (2 → 12 parallel runners)
- Isolate import test resources per run via RESOURCE_SUFFIX to prevent concurrent conflicts
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

/strands review

@HweinstockHweinstock changed the title ci: cut e2e time in half via vitest shardingci: cut full e2e time in half via vitest shardingApr 30, 2026
@Hweinstock
Hweinstock marked this pull request as ready for review April 30, 2026 13:36
@Hweinstock
Hweinstock requested a review from a teamApril 30, 2026 13:36
tejaskash
tejaskash previously approved these changes Apr 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Code Review Summary

I've reviewed PR #1016, which adds 6-way vitest sharding to reduce E2E test time from 19-25 min to ~11.5 min. The approach is sound and well-measured, but there is one blocking issue that must be fixed before merge.


🔴 Blocking Issues

Missing import in cleanup_resources.py

File:e2e-tests/fixtures/import/cleanup_resources.py
Lines: 15, 25

The script uses RESOURCE_SUFFIX on line 25 but doesn't import it from common.py on line 15:

# Line 15 - current importsfromcommonimportREGION, RESOURCES_FILE, get_control_client, get_account_id# Line 25 - uses RESOURCE_SUFFIX (not imported!)prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""

Impact: This will cause a NameError: name 'RESOURCE_SUFFIX' is not defined when cleanup runs after a test, potentially leaving resources in AWS.

Fix: Add RESOURCE_SUFFIX to the import statement:

fromcommonimportREGION, RESOURCES_FILE, RESOURCE_SUFFIX, get_control_client, get_account_id

💡 Suggestions (non-blocking)

1. Resource suffix collision risk

Files:e2e-tests/import-resources.test.ts:37, e2e-tests/import-gateway.test.ts:34

Both tests generate the suffix using Date.now():

constsuffix=Date.now().toString().slice(-8);

In a sharded run, if two shards start within the same millisecond and run the same test, they could collide on resource names. While unlikely, using a more unique identifier would be more robust:

constsuffix=`${Date.now()}-${randomUUID().slice(0,8)}`;

Or include the shard number if available in the environment.


2. S3 cleanup safety concern

File:e2e-tests/fixtures/import/cleanup_resources.py:25

When RESOURCE_SUFFIX is empty or unset, the prefix becomes "", which means list_objects_v2 will return all objects in the bucket:

prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""list_args= {"Bucket": bucket_name}
ifprefix: # Empty string is falsy, so this branch is skippedlist_args["Prefix"] =prefixresp=s3.list_objects_v2(**list_args) # Lists ALL objects

This could accidentally delete objects from other concurrent runs if the env var is missing. Consider:

  1. Making RESOURCE_SUFFIX required (fail fast if unset)
  2. Or use a fallback like "default" instead of empty string

3. Documentation: Why 6 shards?

File:.github/workflows/e2e-tests-full.yml:30

The PR description explains the measured results for 4, 6, and 8 shards, and that 6 is optimal. Consider adding a brief comment in the workflow file explaining why 6 was chosen:

matrix:
cdk-source: [npm, main]# 6 shards provides optimal balance (~11.5 min wall-clock)# 8 shards showed no improvement due to memory deploy (274s) and container build (204s) bottlenecksshard: ['1/6', '2/6', '3/6', '4/6', '5/6', '6/6']

❓ Questions

Concurrency increase: 2 → 12 parallel deploys

File:.github/workflows/e2e-tests-full.yml:28-30

The PR moves from 2 concurrent runners (2 cdk-source entries) to 12 (2 × 6 shards). The description mentions testing 2 concurrent deploys locally but not 12.

Question: Have you verified that the AWS account's CloudFormation/AgentCore control plane can handle 12 concurrent stacks without throttling? The bot comment mentioned this could be "worth finding out empirically," but it would be good to confirm if you've already done a test run with the full matrix.


✅ Positive Aspects

  • Well-measured performance improvement: Clear data showing 19-25 min → 11.5 min
  • Proper resource isolation: Using RESOURCE_SUFFIX to scope S3 objects and resources files per run prevents conflicts between npm and main matrix entries
  • No production code changes: Only CI and test infrastructure, easy to revert if issues arise
  • Comprehensive testing: The PR description shows you tested 4, 6, and 8 shards to find the optimal configuration

Next Steps

  1. Fix the blocking import issue in cleanup_resources.py
  2. Consider the suggestions above (especially the S3 cleanup safety concern)
  3. Confirm the 12 concurrent deploys won't throttle in CI

Overall, this is a great performance improvement with a solid approach. Once the import bug is fixed, this should be good to merge! 🚀

@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

Addressed comments and re ran https://github.com/aws/agentcore-cli/actions/runs/25175265881.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sPR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Hweinstock@agentcore-cli-automation@tejaskash
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci: cut full e2e time in half via vitest sharding by Hweinstock · Pull Request #1016 · aws/agentcore-cli · GitHub
Skip to content

ci: cut full e2e time in half via vitest sharding - #1016

Merged
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard
Apr 30, 2026
Merged

ci: cut full e2e time in half via vitest sharding#1016
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard

Conversation

@Hweinstock

@HweinstockHweinstock commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Description

The full E2E suite takes 19-25 min because all 17 test files run on a single runner per CDK source. Each test deploys its own CloudFormation stack, and measured timings show parallel deploys have zero degradation (77s stays 77s with concurrent stacks).

This adds 6-way vitest sharding via the GitHub Actions matrix, going from 2 to 12 parallel runners. Also isolates import test resources per run to prevent conflicts when npm and main matrix entries hit the same AWS account concurrently.

Measured results:

ShardsWall-clockSlowest shard
1 (baseline)19-25 min
4~15 min890s
6~11.5 min691s
8~12.0 min717s

6 shards appears to be the sweet spot. Adding more shards creates diminishing returns since the main bottleneck is the deploy (274s) and container build (204s). This is roughly what the e2e tests take on PRs when running minimal files so we're likely close to how fast we can get it.

Inspired by #989.

Docs: https://vitest.dev/guide/improving-performance.html#sharding

Import test resource isolation

The import-resources.test.ts setup creates AWS resources (runtime, memory, evaluator) via Python scripts, then saves their ARNs to bugbash-resources.json. The cleanup script deletes those resources and also calls cleanup_s3_code_objects() which previously deleted all objects from the shared S3 bucket bugbash-agentcore-code-{account}-{region}.

The npm and main matrix entries are separate runners hitting the same AWS account simultaneously. Both run import-resources.test.ts. When one job's cleanup finishes first, it nuked all S3 objects — including the code.zip the other job uploaded and was still using, causing downstream resource creation failures.

The fix: each run now gets a unique RESOURCE_SUFFIX (the test's timestamp-based suffix). This scopes:

  • The resources file: bugbash-resources-{suffix}.json instead of bugbash-resources.json
  • The S3 prefix: bugbash-{suffix}/code.zip instead of bugbash/code.zip
  • The S3 cleanup: only deletes objects under its own prefix

Related Issue

N/A — CI performance improvement.

Documentation PR

N/A

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe): CI performance — shard E2E tests for faster wall-clock time

Testing

Tried different shard sizes, runs are here: https://github.com/aws/agentcore-cli/actions/runs/25165632571, https://github.com/aws/agentcore-cli/actions/runs/25166733599

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed — looks good to merge.

Verified:

  • vitest run --project e2e --shard=X/4 is a supported combination in vitest 4.x
  • 16 e2e test files distribute across 4 shards (~4 files each); imbalance is acceptable given fail-fast: false and 60-minute timeout
  • Parallel test concerns look fine: cleanupStaleCredentialProviders() uses a 30-min cutoff so it won't touch resources from concurrently running shards; agent names include framework/provider prefixes so they won't collide across files
  • No production code touched; failure mode is just a CI run that surfaces the issue, easy to revert

One thing worth noting (not blocking): your local measurement validated 2 concurrent deploys, but this moves to 8 concurrent (4 shards × 2 cdk-sources). If you see intermittent CloudFormation/AgentCore control-plane throttling in the first few scheduled runs, that'd be the likely cause — but worth finding out empirically rather than preemptively capping.

@github-actionsgithub-actionsBot removed agentcore-harness-reviewing AgentCore Harness review in progress size/xs PR size: XS labels Apr 29, 2026
@github-actionsgithub-actionsBot added the size/s PR size: S label Apr 29, 2026
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
- Add 6-way vitest sharding to the cdk-source matrix (2 → 12 parallel runners)
- Isolate import test resources per run via RESOURCE_SUFFIX to prevent concurrent conflicts
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

/strands review

@HweinstockHweinstock changed the title ci: cut e2e time in half via vitest shardingci: cut full e2e time in half via vitest shardingApr 30, 2026
@Hweinstock
Hweinstock marked this pull request as ready for review April 30, 2026 13:36
@Hweinstock
Hweinstock requested a review from a teamApril 30, 2026 13:36
tejaskash
tejaskash previously approved these changes Apr 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Code Review Summary

I've reviewed PR #1016, which adds 6-way vitest sharding to reduce E2E test time from 19-25 min to ~11.5 min. The approach is sound and well-measured, but there is one blocking issue that must be fixed before merge.


🔴 Blocking Issues

Missing import in cleanup_resources.py

File:e2e-tests/fixtures/import/cleanup_resources.py
Lines: 15, 25

The script uses RESOURCE_SUFFIX on line 25 but doesn't import it from common.py on line 15:

# Line 15 - current importsfromcommonimportREGION, RESOURCES_FILE, get_control_client, get_account_id# Line 25 - uses RESOURCE_SUFFIX (not imported!)prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""

Impact: This will cause a NameError: name 'RESOURCE_SUFFIX' is not defined when cleanup runs after a test, potentially leaving resources in AWS.

Fix: Add RESOURCE_SUFFIX to the import statement:

fromcommonimportREGION, RESOURCES_FILE, RESOURCE_SUFFIX, get_control_client, get_account_id

💡 Suggestions (non-blocking)

1. Resource suffix collision risk

Files:e2e-tests/import-resources.test.ts:37, e2e-tests/import-gateway.test.ts:34

Both tests generate the suffix using Date.now():

constsuffix=Date.now().toString().slice(-8);

In a sharded run, if two shards start within the same millisecond and run the same test, they could collide on resource names. While unlikely, using a more unique identifier would be more robust:

constsuffix=`${Date.now()}-${randomUUID().slice(0,8)}`;

Or include the shard number if available in the environment.


2. S3 cleanup safety concern

File:e2e-tests/fixtures/import/cleanup_resources.py:25

When RESOURCE_SUFFIX is empty or unset, the prefix becomes "", which means list_objects_v2 will return all objects in the bucket:

prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""list_args= {"Bucket": bucket_name}
ifprefix: # Empty string is falsy, so this branch is skippedlist_args["Prefix"] =prefixresp=s3.list_objects_v2(**list_args) # Lists ALL objects

This could accidentally delete objects from other concurrent runs if the env var is missing. Consider:

  1. Making RESOURCE_SUFFIX required (fail fast if unset)
  2. Or use a fallback like "default" instead of empty string

3. Documentation: Why 6 shards?

File:.github/workflows/e2e-tests-full.yml:30

The PR description explains the measured results for 4, 6, and 8 shards, and that 6 is optimal. Consider adding a brief comment in the workflow file explaining why 6 was chosen:

matrix:
cdk-source: [npm, main]# 6 shards provides optimal balance (~11.5 min wall-clock)# 8 shards showed no improvement due to memory deploy (274s) and container build (204s) bottlenecksshard: ['1/6', '2/6', '3/6', '4/6', '5/6', '6/6']

❓ Questions

Concurrency increase: 2 → 12 parallel deploys

File:.github/workflows/e2e-tests-full.yml:28-30

The PR moves from 2 concurrent runners (2 cdk-source entries) to 12 (2 × 6 shards). The description mentions testing 2 concurrent deploys locally but not 12.

Question: Have you verified that the AWS account's CloudFormation/AgentCore control plane can handle 12 concurrent stacks without throttling? The bot comment mentioned this could be "worth finding out empirically," but it would be good to confirm if you've already done a test run with the full matrix.


✅ Positive Aspects

  • Well-measured performance improvement: Clear data showing 19-25 min → 11.5 min
  • Proper resource isolation: Using RESOURCE_SUFFIX to scope S3 objects and resources files per run prevents conflicts between npm and main matrix entries
  • No production code changes: Only CI and test infrastructure, easy to revert if issues arise
  • Comprehensive testing: The PR description shows you tested 4, 6, and 8 shards to find the optimal configuration

Next Steps

  1. Fix the blocking import issue in cleanup_resources.py
  2. Consider the suggestions above (especially the S3 cleanup safety concern)
  3. Confirm the 12 concurrent deploys won't throttle in CI

Overall, this is a great performance improvement with a solid approach. Once the import bug is fixed, this should be good to merge! 🚀

@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

Addressed comments and re ran https://github.com/aws/agentcore-cli/actions/runs/25175265881.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sPR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Hweinstock@agentcore-cli-automation@tejaskash
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); ci: cut full e2e time in half via vitest sharding by Hweinstock · Pull Request #1016 · aws/agentcore-cli · GitHub
Skip to content

ci: cut full e2e time in half via vitest sharding - #1016

Merged
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard
Apr 30, 2026
Merged

ci: cut full e2e time in half via vitest sharding#1016
Hweinstock merged 2 commits into
aws:mainfrom
Hweinstock:ci/e2e-shard

Conversation

@Hweinstock

@HweinstockHweinstock commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Description

The full E2E suite takes 19-25 min because all 17 test files run on a single runner per CDK source. Each test deploys its own CloudFormation stack, and measured timings show parallel deploys have zero degradation (77s stays 77s with concurrent stacks).

This adds 6-way vitest sharding via the GitHub Actions matrix, going from 2 to 12 parallel runners. Also isolates import test resources per run to prevent conflicts when npm and main matrix entries hit the same AWS account concurrently.

Measured results:

ShardsWall-clockSlowest shard
1 (baseline)19-25 min
4~15 min890s
6~11.5 min691s
8~12.0 min717s

6 shards appears to be the sweet spot. Adding more shards creates diminishing returns since the main bottleneck is the deploy (274s) and container build (204s). This is roughly what the e2e tests take on PRs when running minimal files so we're likely close to how fast we can get it.

Inspired by #989.

Docs: https://vitest.dev/guide/improving-performance.html#sharding

Import test resource isolation

The import-resources.test.ts setup creates AWS resources (runtime, memory, evaluator) via Python scripts, then saves their ARNs to bugbash-resources.json. The cleanup script deletes those resources and also calls cleanup_s3_code_objects() which previously deleted all objects from the shared S3 bucket bugbash-agentcore-code-{account}-{region}.

The npm and main matrix entries are separate runners hitting the same AWS account simultaneously. Both run import-resources.test.ts. When one job's cleanup finishes first, it nuked all S3 objects — including the code.zip the other job uploaded and was still using, causing downstream resource creation failures.

The fix: each run now gets a unique RESOURCE_SUFFIX (the test's timestamp-based suffix). This scopes:

  • The resources file: bugbash-resources-{suffix}.json instead of bugbash-resources.json
  • The S3 prefix: bugbash-{suffix}/code.zip instead of bugbash/code.zip
  • The S3 cleanup: only deletes objects under its own prefix

Related Issue

N/A — CI performance improvement.

Documentation PR

N/A

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe): CI performance — shard E2E tests for faster wall-clock time

Testing

Tried different shard sizes, runs are here: https://github.com/aws/agentcore-cli/actions/runs/25165632571, https://github.com/aws/agentcore-cli/actions/runs/25166733599

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed — looks good to merge.

Verified:

  • vitest run --project e2e --shard=X/4 is a supported combination in vitest 4.x
  • 16 e2e test files distribute across 4 shards (~4 files each); imbalance is acceptable given fail-fast: false and 60-minute timeout
  • Parallel test concerns look fine: cleanupStaleCredentialProviders() uses a 30-min cutoff so it won't touch resources from concurrently running shards; agent names include framework/provider prefixes so they won't collide across files
  • No production code touched; failure mode is just a CI run that surfaces the issue, easy to revert

One thing worth noting (not blocking): your local measurement validated 2 concurrent deploys, but this moves to 8 concurrent (4 shards × 2 cdk-sources). If you see intermittent CloudFormation/AgentCore control-plane throttling in the first few scheduled runs, that'd be the likely cause — but worth finding out empirically rather than preemptively capping.

@github-actionsgithub-actionsBot removed agentcore-harness-reviewing AgentCore Harness review in progress size/xs PR size: XS labels Apr 29, 2026
@github-actionsgithub-actionsBot added the size/s PR size: S label Apr 29, 2026
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
- Add 6-way vitest sharding to the cdk-source matrix (2 → 12 parallel runners)
- Isolate import test resources per run via RESOURCE_SUFFIX to prevent concurrent conflicts
@github-actionsgithub-actionsBot added size/s PR size: S and removed size/s PR size: S labels Apr 30, 2026
@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

/strands review

@HweinstockHweinstock changed the title ci: cut e2e time in half via vitest shardingci: cut full e2e time in half via vitest shardingApr 30, 2026
@Hweinstock
Hweinstock marked this pull request as ready for review April 30, 2026 13:36
@Hweinstock
Hweinstock requested a review from a teamApril 30, 2026 13:36
tejaskash
tejaskash previously approved these changes Apr 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Code Review Summary

I've reviewed PR #1016, which adds 6-way vitest sharding to reduce E2E test time from 19-25 min to ~11.5 min. The approach is sound and well-measured, but there is one blocking issue that must be fixed before merge.


🔴 Blocking Issues

Missing import in cleanup_resources.py

File:e2e-tests/fixtures/import/cleanup_resources.py
Lines: 15, 25

The script uses RESOURCE_SUFFIX on line 25 but doesn't import it from common.py on line 15:

# Line 15 - current importsfromcommonimportREGION, RESOURCES_FILE, get_control_client, get_account_id# Line 25 - uses RESOURCE_SUFFIX (not imported!)prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""

Impact: This will cause a NameError: name 'RESOURCE_SUFFIX' is not defined when cleanup runs after a test, potentially leaving resources in AWS.

Fix: Add RESOURCE_SUFFIX to the import statement:

fromcommonimportREGION, RESOURCES_FILE, RESOURCE_SUFFIX, get_control_client, get_account_id

💡 Suggestions (non-blocking)

1. Resource suffix collision risk

Files:e2e-tests/import-resources.test.ts:37, e2e-tests/import-gateway.test.ts:34

Both tests generate the suffix using Date.now():

constsuffix=Date.now().toString().slice(-8);

In a sharded run, if two shards start within the same millisecond and run the same test, they could collide on resource names. While unlikely, using a more unique identifier would be more robust:

constsuffix=`${Date.now()}-${randomUUID().slice(0,8)}`;

Or include the shard number if available in the environment.


2. S3 cleanup safety concern

File:e2e-tests/fixtures/import/cleanup_resources.py:25

When RESOURCE_SUFFIX is empty or unset, the prefix becomes "", which means list_objects_v2 will return all objects in the bucket:

prefix=f"bugbash-{RESOURCE_SUFFIX}/"ifRESOURCE_SUFFIXelse""list_args= {"Bucket": bucket_name}
ifprefix: # Empty string is falsy, so this branch is skippedlist_args["Prefix"] =prefixresp=s3.list_objects_v2(**list_args) # Lists ALL objects

This could accidentally delete objects from other concurrent runs if the env var is missing. Consider:

  1. Making RESOURCE_SUFFIX required (fail fast if unset)
  2. Or use a fallback like "default" instead of empty string

3. Documentation: Why 6 shards?

File:.github/workflows/e2e-tests-full.yml:30

The PR description explains the measured results for 4, 6, and 8 shards, and that 6 is optimal. Consider adding a brief comment in the workflow file explaining why 6 was chosen:

matrix:
cdk-source: [npm, main]# 6 shards provides optimal balance (~11.5 min wall-clock)# 8 shards showed no improvement due to memory deploy (274s) and container build (204s) bottlenecksshard: ['1/6', '2/6', '3/6', '4/6', '5/6', '6/6']

❓ Questions

Concurrency increase: 2 → 12 parallel deploys

File:.github/workflows/e2e-tests-full.yml:28-30

The PR moves from 2 concurrent runners (2 cdk-source entries) to 12 (2 × 6 shards). The description mentions testing 2 concurrent deploys locally but not 12.

Question: Have you verified that the AWS account's CloudFormation/AgentCore control plane can handle 12 concurrent stacks without throttling? The bot comment mentioned this could be "worth finding out empirically," but it would be good to confirm if you've already done a test run with the full matrix.


✅ Positive Aspects

  • Well-measured performance improvement: Clear data showing 19-25 min → 11.5 min
  • Proper resource isolation: Using RESOURCE_SUFFIX to scope S3 objects and resources files per run prevents conflicts between npm and main matrix entries
  • No production code changes: Only CI and test infrastructure, easy to revert if issues arise
  • Comprehensive testing: The PR description shows you tested 4, 6, and 8 shards to find the optimal configuration

Next Steps

  1. Fix the blocking import issue in cleanup_resources.py
  2. Consider the suggestions above (especially the S3 cleanup safety concern)
  3. Confirm the 12 concurrent deploys won't throttle in CI

Overall, this is a great performance improvement with a solid approach. Once the import bug is fixed, this should be good to merge! 🚀

@Hweinstock

Copy link
Copy Markdown
ContributorAuthor

Addressed comments and re ran https://github.com/aws/agentcore-cli/actions/runs/25175265881.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sPR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Hweinstock@agentcore-cli-automation@tejaskash