chore(tests): drop a write-timeout assertion that the line above already decides

[tylerkron]

Produced by the useless-test-pruner maintenance routine, which looks for tests asserting something that structurally cannot fail. This one is a single deleted line, and it is safe because the assertion it removes was already decided by the assertion immediately above it.

What was wrong

SerialStreamTransport_ApplyOperationalTimeouts_BoundsBothDirections checked the serial write timeout twice. It first pinned it to exactly 2000, then asserted it was not SerialPort.InfiniteTimeout. InfiniteTimeout is the compile-time constant -1, so by the time the second assertion runs, the first one has already established the value is 2000 — the "not infinite" check can never be the thing that fails. It reads like a second, independent guarantee about the write path being bounded, but it contributes nothing, and that is worse than not being there at all: it makes the test look like it covers more than it does.

How it was fixed

Deleted that one line. Assert.Equal(2000, port.WriteTimeout) directly above is untouched and is what actually covers the behavior — it is strictly stronger, since pinning the exact value rules out infinity along with every other wrong value. The ReadTimeout assertion and the explanatory comment above the test are also untouched.

The one thing a reviewer might reasonably push back on is whether the deleted line documented the intent of #399. I do not think it did: the regression #399 describes is WriteTimeout keeping the 30-second connect timeout for the life of the port, not it going infinite. The test arranges exactly that scenario (WriteTimeout = 30000) and Assert.Equal(2000, ...) is the assertion that catches it. The prose comment above the test already states the "both directions must end up bounded and short" intent for a future reader.

Verification

Build clean, 0 warnings. Full suite green on both target frameworks (run under a tests.N lock): net9.0 3827 passed / 0 failed / 2 skipped, net10.0 3827 passed / 0 failed / 2 skipped, plus Daqifi.Mcp.Tests 217 passed. Coverage is unchanged — the removed line exercised no production code the surviving assertion does not.

Bench validation does not apply here: this is a test-only change that touches no production code and needs no device, so the board was never used.

Not merging — for review.

[tylerkron]

/agentic_review

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-code-review]

PR Summary by Qodo

Remove redundant SerialPort write-timeout assertion in SerialStreamTransport tests

🧪 Tests 🕐 Less than 10 minutes

AI Description

• Remove an unfailable write-timeout assertion from SerialStreamTransport timeout test.
• Keep exact-value timeout assertions that already cover the bounded-write guarantee.

Diagram

graph TD
  T["SerialStreamTransportTests"] --> A["Timeout assertions"] --> P["SerialPort timeouts"]

  subgraph Legend
    direction LR
    _test["Test"] ~~~ _logic["Assertion logic"] ~~~ _ext{{"System API"}}
  end

Loading

High-Level Assessment

Current approach is optimal: removing a redundant assertion reduces noise while preserving the stronger exact-value check that already excludes infinite/incorrect timeouts.

Files changed (1) +0 / -1

Tests (1) +0 / -1

SerialStreamTransportTests.csDrop redundant WriteTimeout != InfiniteTimeout assertion +0/-1

Drop redundant WriteTimeout != InfiniteTimeout assertion

• Removes an unfailable assertion that WriteTimeout is not SerialPort.InfiniteTimeout. The preceding exact-value assertion (WriteTimeout == 2000) already guarantees this, so test intent remains unchanged with less redundancy.

src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs

[tylerkron]

Qodo-clean, CI green — ready for review.

Shepherd verification (independent of the opening agent)

• Qodo: round 1 on head 7435f59 — Bugs (0) / Rule violations (0) / Requirement gaps (0), and reviewThreads totalCount 0 (zero threads, not merely zero unresolved). Verified on both surfaces at 16:43Z, 16:46Z and 17:05Z against the same head SHA; neither Qodo comment's updated_at moved, so nothing landed late. The GraphQL query was sanity-checked against chore(tests): remove assertions that structurally cannot fail #620, which correctly returns its 2 known qodo threads, so the empty result here is real and not an empty-query artifact. REST cross-check: pulls/652/reviews and pulls/652/comments both 0.
• Deletion audited: SerialStreamTransport_ApplyOperationalTimeouts_BoundsBothDirections is strictly straight-line — no branch, loop, try, or theory data — so Assert.Equal(2000, port.WriteTimeout) runs on the single path and throws on mismatch. The removed Assert.NotEqual(SerialPort.InfiniteTimeout, ...) could therefore only ever be reached with the value already pinned to 2000, and InfiniteTimeout is const int = -1. Unfailable on every path, and the surviving exact-value assertion is strictly stronger, so no coverage is lost. (reliability: SD download against a wedged card hangs indefinitely and ignores cancellation — no consumer-side watchdog can rescue it #399's regression is WriteTimeout keeping the 30 s connect timeout, which is exactly what Equal(2000) catches.)
• Re-validated against current main by test-merge, which matters here: this PR's CI ran at 16:24Z, before chore(build): centralize the shared build properties in Directory.Build.props #647 merged at 16:33Z and turned on TreatWarningsAsErrors. Merging current origin/main (602a1b7) into the PR head merges cleanly, builds with 0 warnings / 0 errors under chore(build): centralize the shared build properties in Directory.Build.props #647's new Directory.Build.props, and the full suite is green on both frameworks: net9.0 3833 passed / 0 failed / 2 skipped, net10.0 3833 passed / 0 failed / 2 skipped, Daqifi.Mcp.Tests 217 passed.
• No sibling open PR (ci: publish the coverage report CI already computes #648, fix(scpi): reject a negative channel on the two DIO producers #649, fix(tests): ask the exchange whether it recognised the reply, not the clock #650) touches SerialStreamTransportTests.cs. Bench validation N/A — test-only change, no production code, board untouched.