Skip to content

test(api-db): Move DB-only API tests from api-core into api-db#2652

Merged
poroh merged 5 commits into
NVIDIA:mainfrom
poroh:tests-refactoring-p15
Jun 16, 2026
Merged

test(api-db): Move DB-only API tests from api-core into api-db#2652
poroh merged 5 commits into
NVIDIA:mainfrom
poroh:tests-refactoring-p15

Conversation

@poroh

@poroh poroh commented Jun 16, 2026

Copy link
Copy Markdown
Contributor
  • Moves DB-only tests out of api-core and into the owning api-db modules.
  • Covers domain creation, rack/switch/power shelf metadata, machine interface IP allocation, machine interface address lookup, and duplicate MAC handling.
  • Adds api-db::test_support::network_segment::admin_segment for shared DB test fixture setup.
  • Removes TestEnv/API harness usage from these tests where direct DB setup is enough.

Related issues

#2001

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Additional Notes

poroh added 5 commits June 16, 2026 11:15
Move machine interface IP allocation tests into api-db and replace API
fixture setup with direct DB test setup for segments and domains.

Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
Relocate domain create/delete/find/update tests from api-core into the
api-db DNS domain module so they live with the database functionality
they exercise.

Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
Move power shelf, rack, and switch metadata tests into their
respective api-db modules so they live alongside the database code
they exercise.

Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
Move the find-by-address test from api-core into the api-db machine
interface address module, replacing the api-core fixture dependency
with direct database setup.

Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
Move the duplicate machine interface MAC test from api-core into the
api-db machine interface module, and add shared test support for
constructing admin network segments.

Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
@poroh poroh requested a review from a team as a code owner June 16, 2026 18:18
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Summary by CodeRabbit

  • Tests
    • Reorganized test infrastructure to use localized database state initialization instead of shared fixtures.
    • Added new test support utilities for network segment configuration in tests.
    • Refactored test modules across multiple components to improve test isolation and maintainability.

Walkthrough

A new crate-internal test_support module is introduced in api-db with an admin_segment helper that constructs NewNetworkSegment values inline. Test module declarations are moved from the api-core test index into their owning api-db source files, and affected tests are rewritten to begin transactions directly from PgPool and initialize state inline rather than via shared create_test_env fixtures.

Changes

Test Infrastructure Refactor

Layer / File(s) Summary
New test_support::admin_segment helper
crates/api-db/src/lib.rs, crates/api-db/src/test_support/mod.rs, crates/api-db/src/test_support/network_segment.rs
Registers mod test_support behind #[cfg(test)] in lib.rs, declares the pub(crate) mod network_segment submodule, and implements admin_segment(name, prefix, gateway, num_reserved) -> NewNetworkSegment with Admin type, dynamic allocation, a generated UUID, and a single NewNetworkPrefix.
Co-locate test module declarations in api-db
crates/api-core/src/tests/mod.rs, crates/api-db/src/dns/domain.rs, crates/api-db/src/machine_interface.rs, crates/api-db/src/machine_interface_address.rs, crates/api-db/src/power_shelf.rs, crates/api-db/src/rack.rs, crates/api-db/src/switch.rs, crates/api-db/src/dns/domain/test_create_domain.rs, crates/api-db/src/power_shelf/test_metadata.rs, crates/api-db/src/rack/test_metadata.rs, crates/api-db/src/switch/test_metadata.rs
Removes create_domain, ip_allocator, machine_interface_addresses, power_shelf_metadata, rack_metadata, and switch_metadata from the api-core test index. Adds #[cfg(test)] mod ... declarations in each corresponding api-db source file. Inserts use crate as db; aliases into the existing test files that reference db:: paths.
Rewrite tests to use direct transaction setup
crates/api-db/src/machine_interface/ip_allocator.rs, crates/api-db/src/machine_interface/test_duplicate_mac.rs, crates/api-db/src/machine_interface_address/test_find_by_address.rs
All four ip_allocator tests (ipv4, falls_through, ipv6, dual-stack) now call pool.begin() directly and create DNS domains via a local init_dwrt1_domain helper and network segments via admin_segment. test_duplicate_mac replaces create_test_env with direct admin_segment + db::network_segment::persist, a fixed MAC (52:54:00:12:34:56), and a local test_machine_id() helper. test_find_by_address similarly drops create_test_env in favor of pool.begin() plus a local init_dwrt1_domain.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the primary change: relocating database-only tests from api-core to api-db modules, which aligns perfectly with the extensive test reorganization across the changeset.
Description check ✅ Passed The description provides relevant context about the test migration effort, highlights the key functional areas covered, and references the related issue (#2001), demonstrating clear intent aligned with the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
crates/api-db/src/machine_interface/test_duplicate_mac.rs (1)

73-73: ⚡ Quick win

Prefer rollback() for consistency with other tests.

This test validates duplicate MAC detection (error handling) rather than the commit path. Other tests in this PR consistently use txn.rollback().await? (see power_shelf/test_metadata.rs lines 53/90/141/187 and rack/test_metadata.rs lines 45/80/125/166). Using rollback() here clarifies that we're testing error detection without persisting test data.

🔄 Suggested change
-    txn.commit().await?;
+    txn.rollback().await?;
🤖 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-db/src/machine_interface/test_duplicate_mac.rs` at line 73,
Replace the `txn.commit().await?` call with `txn.rollback().await?` in the
test_duplicate_mac test to maintain consistency with other tests in the codebase
that also use rollback() when testing error handling paths. This clarifies that
the test is validating error detection rather than the commit path, and ensures
test data is not persisted.
🤖 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-db/src/machine_interface/test_duplicate_mac.rs`:
- Line 73: Replace the `txn.commit().await?` call with `txn.rollback().await?`
in the test_duplicate_mac test to maintain consistency with other tests in the
codebase that also use rollback() when testing error handling paths. This
clarifies that the test is validating error detection rather than the commit
path, and ensures test data is not persisted.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8c1df1a1-28b3-4970-8572-8c97fe9edf20

📥 Commits

Reviewing files that changed from the base of the PR and between fa88401 and c3b5b6f.

📒 Files selected for processing (17)
  • crates/api-core/src/tests/mod.rs
  • crates/api-db/src/dns/domain.rs
  • crates/api-db/src/dns/domain/test_create_domain.rs
  • crates/api-db/src/lib.rs
  • crates/api-db/src/machine_interface.rs
  • crates/api-db/src/machine_interface/ip_allocator.rs
  • crates/api-db/src/machine_interface/test_duplicate_mac.rs
  • crates/api-db/src/machine_interface_address.rs
  • crates/api-db/src/machine_interface_address/test_find_by_address.rs
  • crates/api-db/src/power_shelf.rs
  • crates/api-db/src/power_shelf/test_metadata.rs
  • crates/api-db/src/rack.rs
  • crates/api-db/src/rack/test_metadata.rs
  • crates/api-db/src/switch.rs
  • crates/api-db/src/switch/test_metadata.rs
  • crates/api-db/src/test_support/mod.rs
  • crates/api-db/src/test_support/network_segment.rs
💤 Files with no reviewable changes (1)
  • crates/api-core/src/tests/mod.rs

@github-actions

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
boot-artifacts-aarch64 3 0 0 3 0 0
boot-artifacts-x86_64 3 0 0 3 0 0
forge-admin-cli-x86_64 244 4 22 103 6 109
machine-validation-runner 671 23 178 262 37 171
machine_validation 671 23 178 262 37 171
nvmetal-carbide 671 23 178 262 37 171
TOTAL 2263 73 556 895 117 622

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

@poroh poroh merged commit bd899c6 into NVIDIA:main Jun 16, 2026
98 of 100 checks passed
@poroh poroh deleted the tests-refactoring-p15 branch July 2, 2026 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants