⚡ perf(test): share CloudFormation stack across integration and benchmark tests #305
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add module-scoped fixtures for LocalStack integration tests to avoid creating/deleting DynamoDB tables for every test function. Changes: - Add localstack_repo_module and localstack_limiter_module fixtures with module scope and loop_scope="module" for proper async event loop handling - Add unique_entity_prefix fixture for data isolation within shared table - Migrate 15 tests across 5 classes to use module-scoped fixtures - Add loop_scope="module" to all test markers using module-scoped fixtures - Keep TestRepositoryLocalStackCloudFormation and TestBucketTTLDowngrade function-scoped as they test stack creation or need separate limiters Performance improvement: ~68s (from ~15 min estimated for the original per-test table creation pattern). Co-Authored-By: Claude Opus 4.5 <[email protected]>
Documents the decision to use module-scoped pytest fixtures for integration tests to reduce test runtime from ~15 min to ~68 sec. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Documents the loop_scope requirement for pytest-asyncio when using module or class-scoped async fixtures to prevent "different loop" errors. Co-Authored-By: Claude Opus 4.5 <[email protected]>
…marks (#253) Add module-scoped sync_localstack_limiter_module fixture for benchmarks that don't require fresh infrastructure. Migrated to module-scoped: - TestLocalStackBenchmarks (2 tests) - TestLocalStackLatencyBenchmarks (4 tests) - TestCascadeOptimizationBenchmarks (3 tests) Kept function-scoped (need fresh infrastructure): - TestLocalStackOptimizationComparison (compares cached vs non-cached) - TestLambdaColdStartBenchmarks (measures Lambda cold start latency) Performance: 9 tests now run in ~35s vs estimated ~280s with per-test stacks. Co-Authored-By: Claude Opus 4.5 <[email protected]>
sodre
added
testing
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: e54de35 | Previous: 894b909 | Ratio |
|---|---|---|---|
tests/benchmark/test_operations.py::TestTransactionOverheadBenchmarks::test_transactional_acquire |
91.26241284510598 iter/sec (stddev: 0.023760188426268092) |
109.58775063190676 iter/sec (stddev: 0.009500152952022072) |
1.20 |
tests/benchmark/test_operations.py::TestConcurrentThroughputBenchmarks::test_same_entity_sequential |
7.8877640231249355 iter/sec (stddev: 0.06221615008565724) |
16.141638669585486 iter/sec (stddev: 0.008738927130837062) |
2.05 |
tests/benchmark/test_operations.py::TestOptimizationComparison::test_cascade_cache_disabled |
30.530195502837074 iter/sec (stddev: 0.04987519780149153) |
36.85450606785011 iter/sec (stddev: 0.019255101845582715) |
1.21 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #305 +/- ##
===========================================
+ Coverage 37.35% 95.71% +58.36%
===========================================
Files 24 24
Lines 4645 4645
===========================================
+ Hits 1735 4446 +2711
+ Misses 2910 199 -2711
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Disable pytest-xdist parallelization for integration tests to maximize the benefit of module-scoped fixtures (ADR-121). With xdist enabled, each worker creates its own CloudFormation stack, limiting the time savings from shared infrastructure. Benchmarks already use -n 0 for the same reason. Issue: #253 Co-Authored-By: Claude Opus 4.5 <[email protected]>
Disabling xdist (-n 0) made integration tests ~3x slower (~26 min vs ~10 min), confirming that parallelization benefits outweigh stack creation overhead. Add --durations=10 to show the 10 slowest tests for analysis. Issue: #253 Co-Authored-By: Claude Opus 4.5 <[email protected]>
Update ADR-121 to reflect measured CI performance: - ~20% improvement (13 min → 10 min), not the initially estimated 75%+ - Document that xdist parallelization limits fixture sharing benefits - Add "Disable xdist" as rejected alternative (26 min sequential) - Clarify the observed limitation with 4 xdist workers Issue: #253 Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Import shared fixtures from integration conftest in E2E conftest - Add module-scoped e2e_limiter_module and e2e_limiter_minimal_module - Update E2E test classes to use module-scoped fixtures with unique_entity_prefix - Change benchmark sync_localstack_limiter_with_aggregator to class-scoped - Update Lambda cold start benchmarks to use unique_entity_prefix Expected savings: ~246s in benchmark cold start tests (4 stacks → 1 stack) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Resource names must start with a letter, but unique_entity_prefix is a hex string that can start with a digit. Prefix with 'r-' to ensure validity. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove TestE2ECloudFormationStackVariations (redundant with other tests) - Merge TestE2ERoleNaming tests into single test_role_naming_formats - Reduces test count from 19 to 16, eliminating ~180s of stack deployment Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove 6 test classes, replace with section comments - Move cli_runner fixture to module level (single definition) - Reduces nesting and simplifies structure - No functional changes (same 16 tests) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add cli_stack_module fixture (module-scoped) - Update test_audit_list_cli_workflow to use shared stack - Update test_list_cli_workflow to use shared stack - Update test_usage_list_plot_cli_workflow to use shared stack - Saves ~210s by deploying 1 stack instead of 3 Co-Authored-By: Claude Opus 4.5 <[email protected]>
…unctions Remove class wrappers from integration tests, consistent with E2E test structure. Uses module-level functions with section comments for organization, keeping module-scoped fixtures for shared infrastructure. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
--durations=10to CI integration tests for ongoing performance analysisPerformance Analysis
Key finding: Parallelization benefit (4 workers) outweighs stack creation overhead. Disabling xdist to maximize fixture sharing made tests ~3x slower.
Improvement: ~20% reduction in integration test time. While modest, the change also:
loop_scoperequirementTest plan
Closes #253
🤖 Generated with Claude Code