[tylerkron]

Summary

Several Daqifi.Core.Tests facts re-asserted exactly what a sibling fact (or a census/guard theory) already pins, so each behavior was tested twice and every future change had to be updated in two places. This removes the duplicates; every removal below names the surviving test that still covers it, and each claim was re-checked against the current origin/main tests.

Deletes (or trims) Daqifi.Core.Tests facts whose assertions are already made by a sibling. No production code. Does not touch files owned by #770, #771, #781, #790, or #805–#814, and does not rework StreamMessageConsumerIntegrationTests.

Touched classes: 416 passed / 0 failed on net9.0 and on net10.0 (Release).

SD card

Removed	Still covered by
ParseFileList_WithJsonLogFileName_ParsesDate	ParseFileList_WithLogFileName_ParsesDate asserts the exact DateTime for the same 20240115_103000 stem. TryParseDateFromLogFileName strips the extension before parsing, so .json is that path. ParseFileList_WithMultipleFormats_RetainsCorrectFileName keeps the .json name and a non-null CreatedDate.
ParseFileList_WithCsvLogFileName_ParsesDate	Same pair, for .csv.
ParseFileList_WithScpiError_SkipsErrorLines	ParseFileList_WithVariousScpiErrorFormats_SkipsAll already includes **ERROR: -200, "Execution error". The mixed-with-files fact stays.
ParseFileList_WithPlainErrorLine_SkipsErrorLine	ParseFileList_WithVariousPlainErrorFormats_SkipsAll already includes ERROR: -200, "Execution error". The mixed-with-files fact stays.
ParseAsync_WithNonPositiveBufferSize_ThrowsArgumentOutOfRange (BufferSize 0)	SdCardParseOptionsBufferSizeGuardTests.ParseEntryPoint_WithNonPositiveBufferSize_ThrowsTheSameArgumentOutOfRange row SdCardFileParser.ParseAsync / 0, which also pins the parameter name and message.
ParseAsync_WithNegativeBufferSize_ThrowsArgumentOutOfRange (BufferSize -1)	Same theory, row SdCardFileParser.ParseAsync / -1.
ReceiveAsync_EofMarkerIsStrippedFromOutput	ReceiveAsync_CompleteFileWithEofMarker_WritesCorrectBytes requires the destination bytes to equal the payload, so the marker cannot be in the output. The transfer-error strip fact stays.
Assert.True(p.BytesReceived >= 0) inside ReceiveAsync_ProgressReporting_BytesReceivedIncreases	The non-empty report list and the file name stay. Exact counts are ReceiveAsync_CompleteFileWithEofMarker_WritesCorrectBytes and ReceiveAsync_MarkerOnlyTransferForListedEmptyFile_ReportsProgress (BytesReceived == 0).
ScaleRawAnalogValues_Double_LeavesTheOffsetOutOfTheInternalScale	ScaleRawAnalogValues_Double_ReadsEachChannelsOwnCalibrationRangeAndScale expects 200/1000 * 20 * 3 * 2 - 0.25 on the same PerChannelConfig / RawCounts. That is 24 - 0.25. Scaling the intercept by the internal scale would be 23.5 and fail it.
ParseAsync_ChannelCountInferredFromColumnHeader	ParseAsync_ExtractsDeviceConfigFromCommentHeaders asserts AnalogPortCount == 3. There is no port-count comment; both files are three analog columns from BuildCsvFileSharedTimestamp, and the parser counts those pairs.
ParseAsync_IntegerAnalogValues_ParsesCorrectly	ParseAsync_WithoutCalibrationConfig_ReturnsRawValues feeds integer JSON (22, 16) through the same GetDouble path and asserts the raw doubles.
Assert.True(p.BytesRead >= 0) inside ParseAsync_ProgressReporting_CallsCallback	progressCalls >= 2 and MessagesRead == 250 stay.

Firmware version

Removed	Still covered by
CompareTo_NewerVersion_ReturnsPositive	> is CompareTo > 0. CompareTo_MinorTakesPrecedence (also major, patch, and the pre-release facts) asserts >. Compare_Strings_BothValid asserts Compare("3.2.0", "3.1.0") > 0, which calls CompareTo.
CompareTo_OlderVersion_ReturnsNegative	< would otherwise have been unasserted, so Operators_LessThanOrEqual_GreaterThanOrEqual now also asserts c < a beside c <= a. < is CompareTo < 0. Compare_Strings_BothValid asserts the other direction of Compare.

Guards

Removed or trimmed	Still covered by
DeviceConnectionOptions_DefaultValues_AreCorrect	DeviceConnectionOptions_Default_ReturnsDefaultOptions. Default is new(), and that fact asserts the same name, null retry, InitializeDevice, and PreserveActiveStream.
Constructor_NegativeChannelNumber_Throws	ChannelGuardCensusTests site AnalogOutputChannel..ctor(channelNumber) constructs new AnalogOutputChannel(-1) and pins the parameter name, actual value, and message.
Constructor_ImplausibleResolution_Throws rows 0 and -4	Census site resolutionBits: 0 is the same < MinResolutionBits arm (is < 1 or > 32). Row 33 stays — that is > MaxResolutionBits, and the census does not pass it.
Constructor_ImplausibleRangeEndpoint_Throws maximum 100 → 10	Not a deletion. 100 is above MaxRangeMagnitudeVolts (50), so the row threw on the maximum and never showed that the minimum was checked. 10 is legal. NaN, infinity, 60, and -60 stay, as do the empty and inverted range rows.

Communication, timestamp, metadata

Removed	Still covered by
CompositeMessageParser_ParseMessages_WithNoNullBytes_ShouldDetectAsText	CompositeMessageParser_ParseMessages_WithNullTextParser_UsesDefaultLineParser parses the same Hello World\r\n to one string and the same consumed length (a null text parser selects the default line parser). WithTextData_ShouldUseTextParser is the no-null path on new CompositeMessageParser(). WithOneNullByteInPrintableData and ReturnsDifferentMessageTypes stay.
HidDevicePathParser_ParseInstanceId_EmptyInput_ReturnsNull and …_NullInput_ReturnsNull	Collapsed to HidDevicePathParser_ParseInstanceId_NullOrEmptyInput_ReturnsNull. Both inputs still run; they share the one IsNullOrWhiteSpace return.
TimestampResult_PropertiesAreSetCorrectly	ProcessTimestamp_FirstMessage_ReturnsIsFirstMessage (first-message flags, zero cycles, zero seconds), ProcessTimestamp_FirstMessage_TimestampIsNow, ProcessTimestamp_SecondMessage_CalculatesElapsedTimeCorrectly (IsFirstMessage false, cycles, seconds), ProcessTimestamp_DetectsRollover_WhenTimestampWrapsAround (WasRollover true).
ProcessTimestamp_ZeroTimestamp_HandledCorrectly	The first-message branch does not read the counter. ProcessTimestamp_FirstMessage_ReturnsIsFirstMessage. ProcessTimestamp_MultipleMessages_AccumulatesTimeCorrectly also starts at 0.
ProcessTimestamp_MaxTimestamp_HandledCorrectly	Same first-message branch. Rollover tests use uint.MaxValue as a previous timestamp, which is a different path.
GetTickPeriod_FrequencySet_ReturnsReciprocalOfFrequency	Reset_PreservesDeviceTimestampFrequency and ResetAll_PreservesDeviceTimestampFrequencies assert GetTickPeriod == 1/10_000_000 after SetTimestampFrequency. SetTimestampFrequency_NonDefaultFrequency_ScalesElapsedTimeCorrectly shows 10,000,000 cycles at 10 MHz is 1 second.
UpdateFromProtobuf_UpdatesHealthTelemetry	UpdateFromProtobuf_FromSerializedStatusPayload_DecodesHealthTelemetry asserts battery 87, power 2, device status 5, and temperature after a protobuf round-trip. Positive temperature is UpdateFromProtobuf_ZeroHealthFields_LeaveLastKnownValues (30); negative is UpdateFromProtobuf_NegativeBoardTemperature_IsPreserved.

Left in place

A sibling does not actually cover these, so they stay:

• Constructor_ImplausibleResolution_Throws for 33 bits.
• Empty/inverted range, and the NaN / infinity / ±60 endpoint rows (maximum corrected).
• WithOneNullByteInPrintableData and ReturnsDifferentMessageTypes.
• SCPI and plain error lines mixed with real files.

Not edited, because an open PR already owns the file:

• Crc16Tests (test: delete redundant and vacuous unit-test facts #814 already deletes Low / High; Constructor_DifferentData_ProducesDifferentCrc and Constructor_SameData_ProducesSameCrc are still there).
• IntelHexParserTests, Pic32BootloaderMessageConsumerTests, Pic32BootloaderMessageProducerTests (test: delete redundant unit-test facts #812; the consumer file is also in test: delete redundant and vacuous unit-test facts #814). Both null-lines facts stay. The framing comment on AllMessages_HaveValidFraming was not touched.
• TcpStreamTransportTests, MessageProducerTests, StreamMessageConsumerTests (test: delete redundant and vacuous unit-test facts #814). The sync disconnect fact is already removed on test: delete redundant and vacuous unit-test facts #814; this PR does not re-delete it.
• StreamMessageConsumerIntegrationTests (test: shared WaitUntil and kill leftover nothing-happened sleeps #771).

Review follow-up

• The #385 rationale comment from the deleted DeviceConnectionOptions_DefaultValues_AreCorrect now sits on the surviving DeviceConnectionOptions_Default_ReturnsDefaultOptions, so the reason PreserveActiveStream defaults to false is not lost.
• Full suite: Core.Tests 4334 passed / 3 skipped on net9.0 and net10.0 (Release).

🤖 Generated with Claude Code

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Remove redundant core tests while preserving coverage

🧪 Tests 🕐 40+ Minutes

AI Description

• Remove duplicate facts where sibling tests or guard theories already cover the same behavior.
• Combine null and empty HID-path cases, and preserve firmware ordering coverage in a surviving
 test.
• Correct the DAC range fixture so invalid-minimum cases cannot pass because of an invalid maximum.

Diagram

graph TD
  Suite["Core test suite"] --> SdTests["SD card tests"] --> SdCore(["SD card behavior"])
  Suite --> ChannelTests["Channel tests"] --> ChannelCore(["Channel guards"])
  Suite --> OtherTests["Other core tests"] --> OtherCore(["Core behaviors"])

Loading

High-Level Assessment

Keep the targeted consolidation: surviving sibling tests and guard theories provide the cited coverage without maintaining duplicate assertions. Retaining every fact would preserve redundancy rather than add a distinct check; the DAC fixture correction is important because it makes the remaining test exercise the intended minimum guard.

Files changed (13) +14 / -316

Tests (13) +14 / -316

AnalogOutputChannelTests.csRemove duplicate guard cases and correct the DAC range fixture +5/-9

Remove duplicate guard cases and correct the DAC range fixture

• Removes a negative-channel fact and resolution cases covered by the channel guard census, retaining the above-maximum resolution case. Changes the range-test maximum to a valid 10 V so invalid-minimum inputs cannot pass by triggering the maximum guard.

src/Daqifi.Core.Tests/Channel/AnalogOutputChannelTests.cs

CompositeMessageParserTests.csRemove a duplicate printable-text parsing fact +0/-17

Remove a duplicate printable-text parsing fact

• Deletes a no-null-byte text case already covered by the line-based parser tests. Other composite-parser cases remain.

src/Daqifi.Core.Tests/Communication/Consumers/CompositeMessageParserTests.cs

DaqifiDeviceFactoryTests.csConsolidate connection-option defaults coverage +2/-15

Consolidate connection-option defaults coverage

• Removes one of two facts asserting the same default options. Moves the historical preserve-active-stream rationale to the surviving fact.

src/Daqifi.Core.Tests/Device/DaqifiDeviceFactoryTests.cs

DeviceMetadataTests.csRemove duplicate health-telemetry mapping test +0/-23

Remove duplicate health-telemetry mapping test

• Deletes the direct protobuf-update fact; the serialized-status-payload test continues to assert the mapped health values.

src/Daqifi.Core.Tests/Device/DeviceMetadataTests.cs

UsbLocationProviderTests.csCombine null and empty HID-path cases +5/-9

Combine null and empty HID-path cases

• Replaces separate null and empty input facts with one parameterized theory that retains both cases.

src/Daqifi.Core.Tests/Device/Discovery/UsbLocationProviderTests.cs

SdCardAnalogScalingTests.csRemove duplicate analog-offset scaling fact +0/-14

Remove duplicate analog-offset scaling fact

• Deletes a separate offset assertion already included in the per-channel calibration and scale test.

src/Daqifi.Core.Tests/Device/SdCard/SdCardAnalogScalingTests.cs

SdCardCsvFileParserTests.csRemove duplicate CSV channel-count fact +0/-16

Remove duplicate CSV channel-count fact

• Deletes the standalone inferred-channel-count fact; the retained header-parsing fact asserts the same analog port count.

src/Daqifi.Core.Tests/Device/SdCard/SdCardCsvFileParserTests.cs

SdCardFileListParserTests.csPrune repeated file-list and error-line cases +0/-56

Prune repeated file-list and error-line cases

• Removes separate JSON and CSV date facts and individual SCPI and plain-error facts. Existing filename/date and error-format tests retain the corresponding cases.

src/Daqifi.Core.Tests/Device/SdCard/SdCardFileListParserTests.cs

SdCardFileParserTests.csRemove repeated binary-parser buffer-size guard facts +0/-28

Remove repeated binary-parser buffer-size guard facts

• Deletes the zero and negative BufferSize facts, which are covered by the shared parse-entry-point guard theory.

src/Daqifi.Core.Tests/Device/SdCard/SdCardFileParserTests.cs

SdCardFileReceiverTests.csTrim duplicate EOF-marker and progress assertions +1/-24

Trim duplicate EOF-marker and progress assertions

• Removes a separate marker-stripping fact because the retained complete-file test checks the exact output bytes. Drops a nonnegative progress-count assertion while retaining progress-report existence and filename checks.

src/Daqifi.Core.Tests/Device/SdCard/SdCardFileReceiverTests.cs

SdCardJsonFileParserTests.csRemove repeated JSON numeric and progress assertions +0/-22

Remove repeated JSON numeric and progress assertions

• Deletes the standalone integer-analog-values fact covered by the numeric-token cases. Removes a nonnegative bytes-read assertion from the progress callback test.

src/Daqifi.Core.Tests/Device/SdCard/SdCardJsonFileParserTests.cs

TimestampProcessorTests.csRemove redundant timestamp-result and edge-case facts +0/-65

Remove redundant timestamp-result and edge-case facts

• Deletes a property-assignment fact, a reciprocal-frequency fact, and first-message checks for zero and maximum timestamps. Surviving tests cover the cited result properties, tick period, and first-message behavior.

src/Daqifi.Core.Tests/Device/TimestampProcessorTests.cs

FirmwareVersionTests.csConsolidate firmware-version ordering assertions +1/-18

Consolidate firmware-version ordering assertions

• Removes separate newer- and older-version comparison facts. Adds a strict less-than assertion to the surviving operator test alongside existing string-comparison coverage.

src/Daqifi.Core.Tests/Firmware/FirmwareVersionTests.cs

[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 add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗