ci: pin nightly toolchain + apply rustfmt-nightly#70
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
| let result = ConfigBuilder::new() | ||
| .load_env("PHENO_CONFIG") | ||
| .build(); | ||
| let result = ConfigBuilder::new().load_env("PHENO_CONFIG").build(); |
There was a problem hiding this comment.
Suggestion: This test now reads process environment variables directly via load_env without any synchronization, while other tests in this crate mutate env vars in parallel. In Rust, concurrent env reads/writes across threads are not safe and can make this test flaky (or worse on some platforms). Serialize env access in this test with the same process-wide lock/guard pattern used by the env-based tests, or avoid env access entirely in this tracing assertion. [race condition]
Severity Level: Major ⚠️
- ⚠️ Tracing integration test `build_emits_span` may become flaky.
- ⚠️ Parallel env-based tests violate std::env concurrency guidance.
- ⚠️ Test suite reliability reduced under multi-threaded test runs.Steps of Reproduction ✅
1. Run the full test suite for the `pheno-config` crate with multiple threads, for
example: `cargo test -p pheno-config --features tracing -- --test-threads=4`. This
executes `tests/config_test.rs`, `tests/toml_merge_test.rs`, and `tests/tracing_test.rs`
in parallel as noted in the comment at
`crates/pheno-config/tests/toml_merge_test.rs:16-18` ("The default cargo test harness runs
tests in parallel worker threads…").
2. In `crates/pheno-config/tests/toml_merge_test.rs:29-63`, the `EnvGuard::set` function
acquires a process-wide lock `ENV_LOCK` (defined at
`crates/pheno-config/tests/toml_merge_test.rs:23`) and then calls `env::remove_var` and
`env::set_var` on various `PHENO_CONFIG_V020_*` environment variables (`KNOWN_V020_VARS`
defined at lines 84-108). These are real writes to the process environment performed while
holding the mutex.
3. Independently, `crates/pheno-config/tests/config_test.rs:35-52` defines another
`EnvGuard` that calls `env::set_var` and `env::remove_var` on `PHENO_CONFIG_IT_*`
variables without any mutex, meaning those tests (e.g.,
`load_from_env_with_prefix_filters_unrelated_vars` at line 85 and
`load_from_env_defaults_port_8080` at line 117) also mutate process environment variables
when run in parallel.
4. While these env-mutating tests are running, the `build_emits_span` tracing test in
`crates/pheno-config/tests/tracing_test.rs:11-19` executes. It calls
`ConfigBuilder::new().load_env("PHENO_CONFIG").build();` at line 16, which reads from the
process environment (via `load_env`) but does not take the `ENV_LOCK` mutex or any other
guard. This creates a real code path where `std::env` reads in `tracing_test.rs` can race
with concurrent `std::env::set_var`/`remove_var` calls in `config_test.rs` and
`toml_merge_test.rs`, matching the race pattern described in the v0.2 comments
(toml_merge_test.rs:16-22). On platforms where concurrent env reads/writes are not safe,
this can lead to flaky behavior in `build_emits_span` (e.g., observing partially-updated
or inconsistent environment state).(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/pheno-config/tests/tracing_test.rs
**Line:** 16:16
**Comment:**
*Race Condition: This test now reads process environment variables directly via `load_env` without any synchronization, while other tests in this crate mutate env vars in parallel. In Rust, concurrent env reads/writes across threads are not safe and can make this test flaky (or worse on some platforms). Serialize env access in this test with the same process-wide lock/guard pattern used by the env-based tests, or avoid env access entirely in this tracing assertion.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
User description
Summary
CodeAnt-AI Description
Pin the Rust toolchain used by CI and formatting checks
What Changed
Impact
✅ Fewer CI failures from toolchain drift✅ Consistent formatting checks✅ More reliable build, lint, and test runs💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.