Skip to content

Improve test quality: CareMetrics.Tests/UnitTests.cs#26

Draft
Copilot wants to merge 2 commits into
mainfrom
copilot/improve-test-quality-caremetrics
Draft

Improve test quality: CareMetrics.Tests/UnitTests.cs#26
Copilot wants to merge 2 commits into
mainfrom
copilot/improve-test-quality-caremetrics

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 5, 2026

UnitTests.cs had 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 of VektisCsvParser.ParseDirectory.

Changes

  • Remove Truth_IsTrue placeholder
  • SolutionRootFixture — extracts the repeated directory-tree-walk into a shared IClassFixture<T>, injected via constructor
  • Split test classesVektisCsvParserTests (stateless unit tests) and VektisServiceIntegrationTests (DI smoke test); was a single mixed class
  • Slim parser test — replaces the ~40-line internal-reimplementation with a direct call + behavioural assertions:
    var records = VektisCsvParser.ParseFile(csvFile);
    Assert.NotEmpty(records);
    Assert.All(records, r => {
        Assert.False(string.IsNullOrEmpty(r.CareType));
        Assert.True(r.InsuredCount > 0);
        Assert.True(r.Year > 2000);
    });
  • ParseFile_VektisFile_HasExpectedColumnCount — pins the 31-column schema via File.ReadLines, no parser duplication
  • ParseDirectory coverage — two new tests: valid directory (multi-file result exceeds single-file result) and empty temp directory returns empty
  • [Theory] edge casesParseFile_InvalidPath_ReturnsEmpty ("", "non_existent_file.csv") and ParseFile_NullPath_ThrowsArgumentNullException document the input contract

Net: 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.cs has 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

Property Value
Test File CareMetrics.Tests/UnitTests.cs
Source File(s) CareMetrics.API/Services/VektisCsvParser.cs, CareMetrics.API/Services/VektisDataService.cs
Test Methods 3 [Fact] methods
Lines of Code 110 lines

Test Quality Analysis

Strengths ✅

  1. Real-data integration testParser_CanParseRealVektisFile walks up to the solution root and exercises the actual CSV parser against live data, catching environment-level issues.
  2. DI wiring testService_CanLoadSampleCsv_FromConfiguration validates that ServiceCollection correctly resolves IVektisDataService and loads multi-year data — a valuable smoke test.
  3. Meaningful assertions — Most assertions include diagnostic messages (e.g. $"Expected >10k records, got {all.Count}") that aid debugging.

Areas for Improvement 🎯

1. Remove Placeholder Test

Current Issue:

// ❌ CURRENT — meaningless, always passes
[Fact]
public void Truth_IsTrue()
{
    Assert.True(true);
}

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_CanParseRealVektisFile and Service_CanLoadSampleCsv_FromConfiguration repeat 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):

// ✅ IMPROVED — shared fixture
public class SolutionRootFixture
{
    public string SolutionRoot { get; }
    public SolutionRootFixture()
    {
        var dir = new DirectoryInfo(AppContext.BaseDirectory);
        while (dir != null && !File.Exists(Path.Combine(dir.FullName, "CareMetrics.slnx")))
            dir = dir.Parent;
        SolutionRoot = dir?.FullName
            ?? throw new DirectoryNotFoundException("CareMetrics.slnx not found");
    }
}

public class UnitTests : IClassFixture(SolutionRootFixture)
{
    private readonly string _solutionRoot;
    public UnitTests(SolutionRootFixture fixture) => _solutionRoot = fixture.SolutionRoot;
    // ...
}

3. Simplify Parser_CanParseRealVektisFile and 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:

// ❌ CURRENT — duplicates parser internals
using var reader = new StreamReader(targetFile);
var headerLine = reader.ReadLine()!;
var headers = headerLine.Split(';').Select(h => h.Trim().ToLowerInvariant()).ToArray();
Assert.Equal(31, headers.Length);
Assert.Equal("geslacht", headers[0]);
// ... 20+ more lines before calling the actual parser

// ✅ IMPROVED — tests parser behaviour, not internals
[Fact]
public void ParseFile_RealVektisFile_ReturnsNonEmptyRecordsWithRequiredFields()
{
    var csvFile = GetSampleCsvFile(); // helper returning a single .csv path

    var records = VektisCsvParser.ParseFile(csvFile);

    Assert.NotEmpty(records);
    Assert.All(records, r =>
    {
        Assert.False(string.IsNullOrEmpty(r.CareType));
        Assert.True(r.InsuredCount > 0);
        Assert.True(r.Year > 2000);
    });
}

If the header layout is critical to pin down, put it in a focused separate [Fact]:

[Fact]
public void ParseFile_VektisFile_HasExpectedColumnCount()
{
    var csvFile = GetSampleCsvFile();
    // Read only the header via File.ReadLines — no parser duplication
    var headers = File.ReadLines(csvFile).First().Split(';');
    Assert.Equal(31, headers.Length);
}

4. Add [Theory] Tests for VektisCsvParser

VektisCsvParser has two public static methods — ParseFile and ParseDirectory — but neither is tested with edge cases. The existing test only covers the "happy path" with a real file.

Missing Tests:

// ✅ ADD — edge cases for ParseFile
[Theory]
[InlineData(null)]
[InlineData("")]
[InlineData("non_existent_file.csv")]
public void ParseFile_InvalidPath_ThrowsOrReturnsEmpty(string? path)
{
    // ParseFile should throw for null/empty/missing paths — do...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes rowantervelde/agentic-development#25

<!-- START COPILOT CODING AGENT TIPS -->
---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security)

Co-authored-by: rowantervelde <22254598+rowantervelde@users.noreply.github.com>
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