Skip to content

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

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/improve-test-quality
Draft

Improve test quality: CareMetrics.Tests/UnitTests.cs#33
Copilot wants to merge 2 commits intomainfrom
copilot/improve-test-quality

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 8, 2026

UnitTests.cs had a placeholder test, duplicated solution-root discovery logic, a test that re-implemented CSV parsing internals instead of asserting on parser output, and no coverage of VektisCsvParser.ParseDirectory.

Changes

  • Removed Truth_IsTrue (Assert.True(true)) placeholder
  • Extracted GetSolutionRoot() private helper — eliminates the copy-pasted directory-walk from two test methods
  • Refactored Parser_CanParseRealVektisFile — replaced manual CSV header/column inspection with assertions on the returned VektisRecord list:
    var records = VektisCsvParser.ParseFile(targetFile);
    Assert.True(records.Count > 0, ...);
    Assert.All(records, r => {
        Assert.False(string.IsNullOrEmpty(r.CareType));
        Assert.True(r.Year > 2000);
        Assert.True(r.InsuredCount > 0);
    });
  • Added [Theory]/[InlineData] test covering year extraction for 2020, 2021, and 2023 files
  • Added ParseDirectory_HappyPath_ReturnsRecords — asserts non-empty results spanning multiple years
  • Added ParseDirectory_EmptyDirectory_ReturnsEmpty — covers the empty-dir edge case using a temp directory
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 was 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

Item Value
Test File CareMetrics.Tests/UnitTests.cs
Source File(s) CareMetrics.API/Services/VektisCsvParser.cs
Test Methods 3 [Fact] / 0 [Theory] methods
Lines of Code 110 lines
Uncovered Public Methods VektisCsvParser.ParseDirectory (not tested at all)

Test Quality Analysis

Strengths ✅

  1. Real-data integration testingParser_CanParseRealVektisFile and Service_CanLoadSampleCsv_FromConfiguration exercise the system against actual CSV files on disk, which catches real-world parsing issues.
  2. Detailed failure messages — Assertions include contextual strings (e.g., $"Parser produced 0 records from {Path.GetFileName(targetFile)}") that aid debugging on CI.

Areas for Improvement 🎯

1. Remove placeholder test — Assert.True(true)

Current code (line 13-16):

[Fact]
public void Truth_IsTrue()
{
    Assert.True(true);
}

Problem: This test asserts nothing meaningful. It will always pass regardless of any code change, providing a false sense of test coverage and inflating the test count.

Recommended change: Delete this method entirely. If a "smoke test" is still desired, replace it with something that actually verifies a real behaviour:

// ❌ REMOVE
[Fact]
public void Truth_IsTrue()
{
    Assert.True(true);
}

// ✅ REPLACE WITH something meaningful, e.g.:
[Fact]
public void ParseFile_FileWithNoYear_ReturnsEmpty()
{
    var tempFile = Path.GetTempFileName();
    File.WriteAllText(tempFile, "geslacht;leeftijdsklasse\nM;0-17");
    try
    {
        var result = VektisCsvParser.ParseFile(tempFile); // filename has no year
        Assert.Empty(result);
    }
    finally { File.Delete(tempFile); }
}

Why this matters: Placeholder tests mask coverage gaps and clutter test output. xUnit has no concept of "pending" tests — a passing Assert.True(true) is silent noise.

2. Extract shared helper — duplicate solution-root discovery

Problem: The same 4-line AppContext.BaseDirectory → directory-walk pattern appears verbatim in both Parser_CanParseRealVektisFile (lines 21–27) and Service_CanLoadSampleCsv_FromConfiguration (lines 67–73). Any change to the discovery logic must be made in two places.

Recommended change: Extract a private helper or use a shared fixture:

public class UnitTests
{
    // ── Shared helper ──────────────────────────────────────────────
    private static string GetSolutionRoot()
    {
        var dir = new DirectoryInfo(AppContext.BaseDirectory);
        while (dir != null && !File.Exists(Path.Combine(dir.FullName, "CareMetrics.slnx")))
            dir = dir.Parent;
        Assert.NotNull(dir);
        return dir!.FullName;
    }

    [Fact]
    public void Parser_CanParseRealVektisFile()
    {
        var solutionRoot = GetSolutionRoot();
        // rest of the test uses solutionRoot…
    }
}

Why this matters: DRY reduces the blast radius of future changes (e.g., if the solution file is renamed or the layout changes).

3. Parser_CanParseRealVektisFile re-implements parser internals

Problem: The test manually reads headers and data lines (lines 32–62) to verify assumptions that are already encoded in VektisCsvParser.ParseFile itself. This makes the test:

  • Brittle — it will break if the CSV column layout changes, but the parser already handles that.
  • Redundant — it tests the setup preconditions of the parser rather than the output of the parser.
  • Long — 46 lines for what is conceptually a single assertion.

Recommended refactor: Trust ParseFile to handle the CSV; test only its output:

[Fact]
public void ParseFile_WithReal2023File_ReturnsWellFormedRecords()
{
    var solutionRoot = GetSolutionRoot();
    var csvDir = Path.Combine(solutionRoot, "CareMetrics.API", "Data", "vektis", "postcode3");
    var targetFile = Directory.GetFiles(csvDir, "*2023*.csv").FirstOrDefault()
                     ?? Directory.GetFiles(csvDir, "*.csv").First();

    var records = VektisCsvParser.ParseFile(targetFile);

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

The manual header-sniffing block (bytes, hex, decimal.TryParse) is di...


🔒 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.

Co-authored-by: rowantervelde <22254598+rowantervelde@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve test quality in CareMetrics.Tests Improve test quality: CareMetrics.Tests/UnitTests.cs Mar 8, 2026
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.

[xunit-expert] Improve Test Quality: CareMetrics.Tests/UnitTests.cs

2 participants