feature-unit-testing

name	feature-unit-testing
description	Use when writing, planning, or improving unit tests for low-level transport or systems code — especially when reasoning about branch coverage, test gaps, identifying which uncovered paths are worth pursuing, or deciding when a feature's test suite is ready to merge.

Feature-Centric Unit Testing

Overview

One test = one transport feature. Tests live at the internal API boundary, not at the system level (ncclAllReduce). This localizes failures immediately: broken test → broken feature, no stack archaeology. The suite is the acceptance criterion for every PR.

Central discipline: measure coverage first, plan second. Verify assumptions against real data before writing a single test.

Modules

• multithread-validation.md — covering concurrent use of a net plugin: which production threading to reproduce, which concurrency is not worth testing, worker-option parsing, and how to make overlap an assertion.

---

Feature Testing Lifecycle

Feature spec / integration plan
         │
         ▼
  DOMAIN 1 — Requirements & Planning
  • Write test plan at spec time (before any code)
  • Hardware scope: which cluster, NIC model, GDR, AINIC, QP count
  • Agree on coverage tier as merge acceptance criterion
         │
         ▼
  DOMAIN 2 — Basic Scenarios
  • Happy path, functional correctness, data integrity
  • Whitebox: direct internal API calls, scheduler inspection
  • Typically reaches ~50% line coverage
         │
         ▼
  DOMAIN 3 — Bottleneck & Edge Case Discovery
  • Fault injection, stress, boundary sizes, concurrent connections
  • Parametric sweeps, async data mutation, multidirectional patterns
  • Pushes coverage from 50% toward 70–90%+
         │
         ▼
  ACCEPTANCE — Coverage Measurement
  • Measure → classify gap → add tests → re-measure
  • below target ──► identify gap ──► add tests ──► re-measure
  • at target    ──► merge

TDD analogy: test plan = "red" phase written before code exists; Domain 2+3 = "green"; coverage measurement = objective acceptance signal.

---

Coverage Acceptance Tiers

Prefer branch coverage as the primary merge criterion. Line coverage is a secondary signal; function coverage is informational only. See the Why Branch Coverage section.

Tier	Line coverage	Branch coverage	When to target
Basic	≥ 50%	≥ 35%	New feature, first PR — core path exercised
Standard	≥ 70%	≥ 50%	Feature complete — error paths and main branches covered
Thorough	≥ 90%	≥ 65%	Stable, high-impact — fault injection required
Critical	≥ 95%	≥ 80%	Safety-critical: fatal-error handling, data integrity

Cost of moving between tiers: Standard→Thorough (branch) requires fault injection and parametric sweeps. Thorough→Critical is expensive — hardware-specific paths and rare races; calculate the realistic branch ceiling for your cluster before committing to this tier.

---

Domain 1: Requirements & Planning

Before writing any test code, create a test plan that defines:

1. Feature scope — which internal API surface is being tested, what is explicitly out of scope
2. Scenario inventory — happy path, error paths, edge cases, concurrency patterns
3. Hardware scope — which NIC model, how many ports, GDR/DMA-buf availability, single-node vs cross-host. Discovering that a test requires unavailable hardware after writing it wastes effort.
4. Coverage tier — agree on a branch coverage target (Basic/Standard/Thorough/Critical) as the merge acceptance criterion. This is the "red" phase in TDD: the bar is set before code exists.

The plan becomes the PR description's Test Plan section — reviewable alongside code. It is a planning artifact — do not commit it as a file; keep it in the PR body.

---

Commit Convention

One commit per test. Each test gets its own atomic commit so reviewers can evaluate test intent and scope individually, and bisect targets a single test on regression.

Title format: <subsystem>: add <test name> test (lowercase, no brackets)

net-ib: add test infrastructure (StressTests harness)   ← CMakeLists + base fixture
net-ib: add InvalidRecvCount test                       ← repeated ×N (test name verbatim)
net-ib: update feature-unit-testing skill               ← skill update last

Body (required): one short paragraph — what the test does, what path it covers, BRDA ref:

Call ncclIbIrecv with n=9 (> NCCL_NET_IB_MAX_RECVS=8). Verifies the early-return
branch returns ncclInternalError without crashing (net_ib.cc:2731, BRDA:2731,0,1).

What to NOT commit:

• Test plans (TEST_PLAN.md) — planning artifact, not part of the test suite
• Coverage shell scripts (run_*.sh, merge_coverage.sh) — operational, not source
• Profraw / profdata files — build artifacts

---

Domain 2: Basic Scenario Patterns

Whitebox testing — bypass the public API

Call internal APIs directly. No ncclCommInit overhead. Example:

// Direct internal call — not through ncclAllReduce
IbCastIsend(sendComm, buf, size, tag, mh, nullptr, &req);

// Read scheduler state directly to verify internal invariants
ncclIbCastGetSchedState(sendComm, &state);
EXPECT_GT(state.activeQpTokens[0], 0);

Whitebox lets a test assert that WRR correctly redistributed tokens when one link is slow — something a blackbox allreduce test cannot verify.

Structural patterns (MPI transport tests)

Double-barrier around every send/recv iteration:

// rank 0: post recv  |  rank 1: post send
MPI_Barrier(MPI_COMM_WORLD);   // after both sides post
// both: wait for completion
MPI_Barrier(MPI_COMM_WORLD);   // after both sides complete

Without the second barrier, one rank reuses request slots before the other finishes.

Retry loop for NULL send request (FIFO backpressure is normal):

do {
    ASSERT_EQ(net_->isend(sendComm, data, size, tag, mh, nullptr, &req), ncclSuccess);
    if (req) break;
    usleep(10000);
} while (true);
// Never ASSERT_NE(req, nullptr) immediately after isend.

RAII guard declaration order:

NetConnectionGuard connGuard(net_);          // 1st — connection
auto bufGuard = makeHostBufferAutoGuard(…);  // 2nd — buffers
NetMHandleGuard mhGuard(mh, …);             // 3rd — memory registration
// Destruction runs in reverse: MR deregistered before connection closed.

Non-fatal checks in multi-rank helpers (fan-in, fan-out, all-to-all):

// Use EXPECT_ (non-fatal), not ASSERT_ (fatal), before any MPI_Barrier.
// A fatal assert in rank 0 leaves all other ranks hanging at the barrier.
EXPECT_EQ(PostRecv(…), ncclSuccess);
// …
MPI_Barrier(MPI_COMM_WORLD);  // unconditional — always reached

Timeout in poll loops (never spin forever):

// Poll loops that wait for async completion MUST have a timeout.
// An infinite loop masks the real bug (e.g. unroutable NIC) as "test hung".
auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(30);
while (!comm && std::chrono::steady_clock::now() < deadline) {
    ncclResult_t r = AcceptConnection(listenComm, &comm);
    if (r != ncclSuccess) break;
}
if (!comm) { /* handle timeout — GTEST_SKIP or fail with diagnostic */ }

Track the return value of each poll iteration separately from the NULL-comm check. A loop that only checks !comm cannot distinguish "still in progress" from "API returned an error 10 iterations ago and will never succeed".

GTEST_SKIP() synchronization across ranks:

// When one rank detects a condition requiring SKIP (e.g. QP connect failed),
// it MUST broadcast this to all ranks BEFORE calling GTEST_SKIP().
// GTEST_SKIP() does a return — if rank 0 returns, rank 1 hangs on MPI_Recv.
int skipFlag = (connectFailed ? 1 : 0);
MPI_Allreduce(MPI_IN_PLACE, &skipFlag, 1, MPI_INT, MPI_MAX, MPI_COMM_WORLD);
if (skipFlag) {
    GTEST_SKIP() << "QP connect failed on at least one rank";
}

Without the Allreduce, a unilateral GTEST_SKIP() in one rank leaves the other rank blocked forever at its next MPI call or poll loop.

---

Domain 3: Fault Injection

Deliberately break one component — verify the system response.

Compile-guarded (-DENABLE_FAULT_INJECTION=ON), zero cost in production:

ncclIbCastFaultSetQpDelay(comm, qpIdx, delayUs);   // artificial latency on one QP
ncclIbCastFaultSetQpError(comm, qpIdx, true);       // force ncclSystemError
ncclIbCastFaultClear(comm);                         // reset for recovery test

Test categories fault injection enables:

• Fatal event propagation — all QPs faulted simultaneously
• Single link failure — one QP faulted, WRR steered away deterministically
• Scheduler rebalancing — artificial delay → WRR redistributes tokens
• Data integrity under delay — sends complete correctly despite latency
• Recovery — fault → clear → fresh connection completes cleanly

When fault injection is the only option: wc.status != IBV_WC_SUCCESS paths in ncclIbTest, fatalErrorCount threshold branches, async error processing — all show zero hits in coverage until a fault injection hook exists.

Without a compile-time hook: use LD_PRELOAD to intercept at the dynamic linker level:

// libibverbs_mock.so — intercepts ibv_poll_cq, injects bad WC after N calls
int ibv_poll_cq(struct ibv_cq *cq, int num, struct ibv_wc *wc) {
    static int calls = 0;
    if (++calls == INJECT_AT) { wc->status = IBV_WC_REM_ACCESS_ERR; return 1; }
    return real_ibv_poll_cq(cq, num, wc);
}

LD_PRELOAD only works if the target calls the symbol through the PLT. It does not work when the code uses dlopen/dlsym to resolve symbols into private function pointers, or dispatches through a driver ops-struct (e.g. cq->context->ops.poll_cq) — both bypass interposition. Check the binary for dlopen/dlsym usage before planning a shim; if it's there, fall back to a compile-time fault hook instead.

---

Domain 3: Stress Testing

Resource leak detection:

100 × (listen → connect → transfer → close)
Assert QP / PD / CQ / MR counts return to baseline after every cycle.

Transfer size coverage:

• Minimum: 1-byte messages — exposes framing bugs invisible at larger sizes
• Non-powers-of-two: 3, 7, 1023, 4097, 65537 — alignment and boundary conditions
• Maximum: up to 64 MB — exercises large MR registration and buffer management

Multidirectional patterns: allgather, alltoall, hypercube topologies; bidirectional simultaneous send/recv on the same connection; many concurrent connections open simultaneously (stress QP allocation limits).

Async data mutation: mutate the send buffer after posting the send. Verifies the NIC captured data before mutation — detects use-after-post bugs in buffer registration.

---

Domain 3: Parametric Configuration Matrix

One test body swept over a grid of env-var combinations:

WRR on/off × QPS count × split threshold × adaptive routing threshold

Surfaces interactions between features that per-feature tests miss. Implement with GTEST_SKIP() when a required env var is absent:

const char* v = getenv("NCCL_IB_QPS_PER_CONNECTION");
if (!v) GTEST_SKIP() << "Set NCCL_IB_QPS_PER_CONNECTION to run this test";

---

Why Branch Coverage, Not Lines or Functions

Line coverage lies. Branch coverage tells the truth.

A line is "covered" if it executed at least once — regardless of which path through it was taken. A function is "covered" if it was called — regardless of what happened inside. A branch is covered only when both sides of a conditional have been exercised.

// Line coverage: 100% (line executes in every test)
// Branch coverage: 50% (error path never taken)
if (result != ncclSuccess) goto fail;   // ← "fail" branch: count = 0

Typical pattern in transport code: adding a batch of stress tests can leave line coverage essentially flat while moving branch coverage only marginally — because the new tests re-exercise already-covered happy-path lines, while the uncovered side of each decision (the error or hardware-specific path) stays untaken. A healthy-looking line-coverage figure can hide that a large share of decision points are only half-tested. For transport code that gap maps directly to bugs tests cannot catch.

Functions coverage is the least informative metric for systems code. A function called once with the happy-path arguments appears fully covered even if it contains 20 conditional branches that were never exercised.

Branch coverage in practice

Use --branch-coverage in genhtml:

genhtml --branch-coverage merged.lcov.info -o html/
# Without this flag, the HTML report does not show per-branch hit counts.

lcov vs llvm-cov branch counts differ: the two tools report different branch denominators for the same file (llvm-cov includes C++ exception pseudo-branches that lcov's BRDA parsing excludes). Use one tool consistently within a comparison — mixing denominators makes deltas meaningless. lcov/genhtml is preferred for branch-level HTML drill-down.

Read BRDA lines, not DA lines, when hunting gaps:

DA:1234,5        ← line 1234 executed 5 times (line coverage)
BRDA:1234,0,0,5  ← block 0, branch 0: taken 5 times (covered)
BRDA:1234,0,1,0  ← block 0, branch 1: taken 0 times ← THIS IS THE GAP

A line with DA count > 0 but a BRDA count of 0 on one arm is the exact pattern of "line covered, branch not covered" — the most common source of false confidence.

Agree on a branch coverage tier at planning time (see Coverage Acceptance Tiers above). Branch coverage ceilings are hardware-dependent — establish a realistic ceiling (branches reachable on your hardware) before setting a target.

---

Coverage Analysis Workflow

STOP — mandatory before writing any test to cover a gap:

1. Export BRDA data and find the exact branch at the target line.
2. Confirm the count field is 0. If it is > 0, the branch is already covered — do not write a test.
3. Read ±10 lines of source context to classify the branch (Trivial / Structural / Hardware / Fault injection / Dead).
4. Only then write or run a test.

Skipping this check is the single most common cause of zero-delta test additions.

1. Get the raw BRDA data

llvm-profdata merge -sparse *.profraw -o merged.profdata
llvm-cov export -format=lcov -instr-profile=merged.profdata ./binary \
    -sources src/your_file.cc > merged.lcov.info
genhtml --branch-coverage merged.lcov.info -o html/

# Extract uncovered branches (count == 0)
awk -F: '/^SF:.*your_file/{found=1} found && /^BRDA:/ && $NF==0 \
    {print} /^end_of_record/{found=0}' merged.lcov.info | sort -t: -k2 -n

2. Read source context around every uncovered line

Read ±10 lines. A branch at a given line may be a trivial env-var check or a hardware-only fault path — the line number tells you nothing without context.

3. Verify existing tests before claiming a gap

Anti-pattern: "All tests call PostRecv(n=1), so the nreqs > 1 path is uncovered."

What actually happened: MultiRecv and MultiRecvShuffled already called PostRecv(n=8) with shuffled tag order, covering the FIFO slot scan for r > 0. The BRDA count showed > 0. The assumption was wrong.

Rule: Before adding a test to cover branch X, check the BRDA count for that exact line in the merged profile. If count > 0, it is already covered.

---

Branch Classification

Classify every uncovered branch before deciding whether to test it:

Class	Description	Technique
Trivial	One env var or API param change	Set NCCL_IB_X=1, call with n=9
Structural	State the harness can't easily create	Cross-host run, tc qdisc netem delay
Hardware	Needs specific HW (GDR, >1 NIC, non-loopback)	Different node, or GTEST_SKIP()
Fault injection	Needs a mock/shim returning errors	LD_PRELOAD, compile-time hook
Dead/unreachable	Kernel or driver guarantees make it impossible	Document and skip

Ceiling calculation: count only Trivial + Structural (with infra) + Fault injection. Never include Dead branches in projected delta.

What moves coverage in net_ib.cc (measured on one cluster — verify on yours):

• Multi-QP split (NCCL_IB_SPLIT_DATA_ON_QPS=1) — split alignment branches, QP distribution