chore(test): add shared config test helpers and cover config predicates - #2118
Conversation
dd-octo-sts
Bot
added
area/config
This comment has been minimized.
This comment has been minimized.
Binary Size Analysis (Agent Data Plane)Baseline: faadad5 · Comparison: a1e1bcc · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
fa40de5 to
89ec904
Compare
ec3ffc3 to
80fec81
Compare
tobz
changed the title
tobz
added
the
type/chore
There was a problem hiding this comment.
Pull request overview
This PR adds shared test helpers in saluki-config to reduce duplicated test setup when building GenericConfiguration, and expands unit-test coverage for configuration predicates and env-var remapping behavior across crates.
Changes:
- Added
saluki_config::config_fromandsaluki_config::test_env_lockto standardize test configuration loading and env-var serialization. - Updated multiple test modules to use the shared helpers instead of per-file
ConfigurationLoader::for_testswrappers. - Expanded configuration predicate tests (notably in
agent-data-plane) and added coverage for an MRF disabled-path.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/CLEANUP_PLAN.md | Marks the shared config test-helper foundation task as completed. |
| lib/saluki-config/src/lib.rs | Introduces shared env lock + config_from helper; loader now uses the shared lock. |
| lib/saluki-components/src/destinations/dsd_debug_log/mod.rs | Refactors tests to use config_from. |
| lib/saluki-components/src/config/mrf.rs | Refactors tests to use config_from and adds a disabled-guard test for endpoint override. |
| lib/saluki-components/src/config/mod.rs | Refactors env-var remapper tests to use the shared env lock and adds deterministic precedence test. |
| lib/saluki-components/src/config/cluster_agent.rs | Refactors tests to use config_from. |
| lib/saluki-components/src/config/autoscaling_failover.rs | Refactors tests to use config_from. |
| lib/saluki-components/src/common/datadog/io.rs | Refactors tests to use config_from. |
| bin/agent-data-plane/src/config.rs | Refactors tests to use config_from and adds predicate/timeout coverage scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // All tests that mutate process-wide environment variables while loading configuration serialize against a | ||
| // single shared lock (see `test_env_lock`), so that tests in other modules and crates can't race with the | ||
| // env-var manipulation below. | ||
| let guard = test_env_lock().lock().unwrap(); |
| // Serialize against the single shared lock the configuration loader itself uses, so this process-wide env | ||
| // mutation can't race with other env-mutating configuration tests in this crate or any other. | ||
| let _guard = test_env_lock().lock().unwrap(); | ||
|
|
| fn env_var_not_remapped_when_absent() { | ||
| let _guard = ENV_MUTEX.lock().unwrap(); | ||
| let _guard = test_env_lock().lock().unwrap(); | ||
|
|
| assert!(dp.data_pipelines_enabled()); | ||
| assert!(!dp.metrics_pipeline_required()); | ||
| assert!(!dp.logs_pipeline_required()); | ||
| assert!(dp.traces_pipeline_required()); | ||
| } |
![datadog-prod-us1-5[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
This PR introduces shared test helpers (config_from(), test_env_lock()) and expands test coverage of data-plane config predicates and MRF endpoints. All 9 new pipeline-requirement and 3 stop-timeout tests correctly exercise code branches that were previously untested per the CLEANUP_PLAN, with no production-code logic changes.
📊 Validated against 15 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 89ec904 · What is Autotest? · Any feedback? Reach out in #autotest
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89ec904ee5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (5)Experiments configured
Bounds Checks: ✅ Passed (5)
ExplanationA change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression ( |
jszwedko
left a comment
jszwedko
left a comment
There was a problem hiding this comment.
Agreed with the Copilot comments, but otherwise 👍
80fec81 to
2fb00de
Compare
89ec904 to
5b6d83b
Compare
| pub async fn config_from(file_values: serde_json::Value) -> GenericConfiguration { | ||
| let (config, _) = ConfigurationLoader::for_tests(Some(file_values), None, false).await; | ||
| config | ||
| } |
Add config_from() and test_env_lock() to saluki-config as plain public helpers, so config tests across crates share one loader wrapper and serialize env-var mutation against a single lock instead of hand-rolling per-file copies. Migrate the duplicated per-file loader helpers and consolidate saluki-components' separate DatadogRemapper env mutex onto the shared lock. Add coverage for previously-untested documented behavior: MrfConfiguration's disabled short-circuit, DatadogRemapper's first-write-wins env remapping (via a new from_env_vars test seam), and DataPlaneConfiguration's pipeline-requirement predicates plus stop-timeout default/sum/override/overflow branches. Resolves G2 of the test suite cleanup plan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0f01d80 to
a1e1bcc
Compare
There was a problem hiding this comment.
More details
PR adds shared test helpers for creating component contexts and configurations in tests, eliminating boilerplate duplication. All helpers are properly feature-gated as test-only code. Comprehensive new tests added for DataPlaneConfiguration predicates cover previously untested paths (pipeline requirements, stop-timeout resolution, overflow handling). No production code changes; purely test infrastructure improvements.
🤖 Datadog Autotest · Commit 0f01d80 · What is Autotest? · Any feedback? Reach out in #autotest
Summary
This PR introduces a number of test hlpers for creating
GenericConfigurationas required in various tests, and switches over a number of tests to use these new helpers.Change Type
How did you test this PR?
New and existing unit tests.
References
DADP-2