Improve test quality: CareMetrics.Tests/UnitTests.cs#26
Draft
Copilot wants to merge 2 commits into
Draft
Conversation
Co-authored-by: rowantervelde <22254598+rowantervelde@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Improve test quality in CareMetrics.Tests/UnitTests.cs
Improve test quality: CareMetrics.Tests/UnitTests.cs
Mar 5, 2026
This was referenced Mar 8, 2026
This was referenced Mar 15, 2026
This was referenced Mar 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
UnitTests.cshad a placeholder test that always passed, duplicated solution-root discovery logic in every test, a 40-line test that re-implemented CSV parser internals rather than testing behaviour, and zero coverage ofVektisCsvParser.ParseDirectory.Changes
Truth_IsTrueplaceholderSolutionRootFixture— extracts the repeated directory-tree-walk into a sharedIClassFixture<T>, injected via constructorVektisCsvParserTests(stateless unit tests) andVektisServiceIntegrationTests(DI smoke test); was a single mixed classParseFile_VektisFile_HasExpectedColumnCount— pins the 31-column schema viaFile.ReadLines, no parser duplicationParseDirectorycoverage — two new tests: valid directory (multi-file result exceeds single-file result) and empty temp directory returns empty[Theory]edge cases —ParseFile_InvalidPath_ReturnsEmpty("","non_existent_file.csv") andParseFile_NullPath_ThrowsArgumentNullExceptiondocument the input contractNet: 22 → 27 tests.
Original prompt
This section details on the original issue you should resolve
<issue_title>[xunit-expert] Improve Test Quality: CareMetrics.Tests/UnitTests.cs</issue_title>
<issue_description>The test file
CareMetrics.Tests/UnitTests.cshas been selected for quality improvement by the xUnit Test Quality Expert. This issue provides specific, actionable recommendations to enhance test quality, coverage, and maintainability using xUnit best practices for .NET.Current State
CareMetrics.Tests/UnitTests.csCareMetrics.API/Services/VektisCsvParser.cs,CareMetrics.API/Services/VektisDataService.cs[Fact]methodsTest Quality Analysis
Strengths ✅
Parser_CanParseRealVektisFilewalks up to the solution root and exercises the actual CSV parser against live data, catching environment-level issues.Service_CanLoadSampleCsv_FromConfigurationvalidates thatServiceCollectioncorrectly resolvesIVektisDataServiceand loads multi-year data — a valuable smoke test.$"Expected >10k records, got {all.Count}") that aid debugging.Areas for Improvement 🎯
1. Remove Placeholder Test
Current Issue:
Recommended Change: Delete it entirely or replace it with a useful test (see "Missing Tests" below).
Why this matters: Placeholder tests inflate pass counts while hiding real coverage gaps and violate the principle that every test should be able to fail.
2. Extract Duplicated Helper Logic
Both
Parser_CanParseRealVektisFileandService_CanLoadSampleCsv_FromConfigurationrepeat the same "walk up the directory tree to find the solution root" logic (~6 lines each). This couples tests to file-system layout and makes maintenance harder.Recommended Change: Extract a shared helper or use
IClassFixture(T):3. Simplify
Parser_CanParseRealVektisFileand Add[Theory]Current Issue: The test manually re-implements CSV parsing logic (reading headers, splitting columns, parsing decimals) to "trace what the parser would do". This duplicates production logic and creates a ~40-line test that tests the test setup rather than the parser behaviour.
Recommended Changes:
If the header layout is critical to pin down, put it in a focused separate
[Fact]:4. Add
[Theory]Tests forVektisCsvParserVektisCsvParserhas two public static methods —ParseFileandParseDirectory— but neither is tested with edge cases. The existing test only covers the "happy path" with a real file.Missing Tests: