test(core-api): Site explorer tests refactoring#2230
Conversation
Use direct imports for `sqlx_test` and `PgPool`, and cache the underlay segment in tests to avoid repeated unwraps. Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
Introduce a test-env construction trait for creating SiteExplorer instances and reuse the env-provided test meter instead of allocating per-test meters. Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
Refactor fake machine and power shelf test helpers to use default relay addresses and credentials, reducing repetitive setup across site explorer tests. Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/api-core/src/tests/site_explorer.rs (1)
3282-3292: ⚡ Quick winSimplify unreachable relay-selection branch in switch DHCP setup.
segmentis assigned fromunderlay_segment, so thematchfallback arm is unreachable. Please collapse this to a single explicit relay value to keep the test intent clear and avoid stale branching logic.Suggested simplification
- let segment = underlay_segment; - let response = env .api .discover_dhcp( DhcpDiscovery::builder( bmc_mac.to_string(), - match segment { - s if s == underlay_segment => "192.0.1.1".to_string(), - _ => "192.0.2.1".to_string(), - }, + UNDERLAY_RELAY.to_string(), ) .tonic_request(), )As per coding guidelines, "Prefer simple, explicit code over clever or heavily abstracted code."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/api-core/src/tests/site_explorer.rs` around lines 3282 - 3292, The match expression selecting the relay IP inside the DhcpDiscovery::builder call is redundant because `segment` is set to `underlay_segment`, making the fallback arm unreachable; replace the match with a single explicit relay string (e.g., "192.0.1.1".to_string()) when calling env.api.discover_dhcp/DhcpDiscovery::builder (the variables to locate are `segment`, `underlay_segment`, `bmc_mac`, and the call site `env.api.discover_dhcp(...)`) to simplify the test and remove the stale branching logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/api-core/src/tests/site_explorer.rs`:
- Around line 3282-3292: The match expression selecting the relay IP inside the
DhcpDiscovery::builder call is redundant because `segment` is set to
`underlay_segment`, making the fallback arm unreachable; replace the match with
a single explicit relay string (e.g., "192.0.1.1".to_string()) when calling
env.api.discover_dhcp/DhcpDiscovery::builder (the variables to locate are
`segment`, `underlay_segment`, `bmc_mac`, and the call site
`env.api.discover_dhcp(...)`) to simplify the test and remove the stale
branching logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fb341e52-b0c7-4ffd-8541-b6d69140f7e4
📒 Files selected for processing (1)
crates/api-core/src/tests/site_explorer.rs
Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
2 participants
Description
This PR contains series of commits that clean up site explorer code.
More clean code will be easier to move to site explorer crate. That I'll do in next PR.
Each individual commit is the PR contains uniform changes.
Type of Change
Related Issues (Optional)
#2001
Breaking Changes
Testing
Additional Notes