test(traces): dedupe test preamble in trace_processor tests - #1364
Open
lucaspimentel wants to merge 3 commits into
Open
test(traces): dedupe test preamble in trace_processor tests#1364lucaspimentel wants to merge 3 commits into
trace_processor tests#1364lucaspimentel wants to merge 3 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
lucaspimentel
changed the title
lucaspimentel
changed the title
trace_processor tests
lucaspimentel
changed the title
trace_processor teststrace_processor tests
Extract repeated Config, tags-provider, ServerlessTraceProcessor, and TracerHeaderTags construction from eight trace-processor tests into shared test helpers. No behavior change; the enriched-payload body-size test keeps its default configuration with local stats disabled.
The helpers are used by tests beyond the error sampler ones, so name them after what they build instead, matching the existing create_* convention in the module. 🤖
lucaspimentel
force-pushed
the
lpimentel/dedupe-trace-processor-test-preamble
branch
from
September 9, 2026 14:32
198f665 to
7d920b4
Compare
lucaspimentel
marked this pull request as ready for review
September 10, 2026 14:27
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is a test-only refactor with no behavioral changes to production code, and the identified issue is a minor maintainability nit.
Pull request overview
Refactors ServerlessTraceProcessor unit tests in trace_processor.rs by extracting repeated test setup into shared helper functions within mod tests, reducing duplication while keeping production behavior unchanged.
Changes:
- Added shared test helpers to construct common
Config,Provider/ServerlessTraceProcessor, andTracerHeaderTagsvalues. - Updated the affected tests to use the new helpers, preserving the one test that intentionally uses the default (non-compute-stats) configuration.
File summaries
| File | Description |
|---|---|
| bottlecap/src/traces/trace_processor.rs | Test-only refactor: deduplicates repeated trace processor test preamble via shared helpers. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Overview
Test-only refactor deferred from #1320 to keep that PR small.
Eight tests in
traces/trace_processor.rsrepeated the same setup: aConfig, aProvider, aServerlessTraceProcessorwith anObfuscationConfig, and aTracerHeaderTagsvalue. This extracts that preamble into three shared test helpers insidemod tests:create_compute_stats_config(): the shared config withlambda_extension_compute_stats: trueused by seven of the testscreate_test_processor(&Arc<Config>, Arc<std::sync::Mutex<ErrorsSampler>>): builds the tags provider (lambdaruntime,test-arnmetadata) and the processor, taking the config and sampler as parameterscreate_test_header_tags(): the shared'statictracer header tagsAccepting the config as a parameter lets
test_process_traces_body_size_reflects_enriched_payloadkeep its deliberate default configuration (local stats disabled) while sharing the rest of the setup. Sampler choices are unchanged: six tests use the enabled error sampler, two use the disabled one. No production code changes.Testing
cargo test -p bottlecap traces::trace_processor::tests: same test count as before the refactor, all passingcargo test -p bottlecap: all unit and integration tests passcargo fmt --all -- --check,cargo clippy --lib --tests, andgit diff --checkclean