Skip to content

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

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

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

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 11, 2026

UnitTests.cs contained a placeholder test, a 44-line monolithic parser test that mixed concerns, and zero coverage for VektisCsvParser.ParseDirectory.

Changes

  • Removed Truth_IsTrue (Assert.True(true)) placeholder
  • Decomposed Parser_CanParseRealVektisFile into two focused tests:
    • Parser_CsvHeaderHasExpectedColumns — validates CSV column contract
    • Parser_ParseFile_ReturnsNonEmptyRecords — validates parser output
  • Added [Theory] edge-case tests for ParseFile — no year in filename and empty file both return []
  • Added Parser_ParseDirectory_EmptyDirectory_ReturnsEmpty — first coverage for ParseDirectory
  • Extracted shared FindVektisCsvFile() helper with null/empty guards to eliminate duplicated directory-walking logic
  • Moved Service_CanLoadSampleCsv_FromConfiguration → new VektisDataServiceIntegrationTests class (renamed Service_LoadsRealCsvData_FromConfiguration) to separate filesystem-dependent integration tests from unit tests
  • Used Path.GetRandomFileName() for temp dirs in edge-case tests to prevent parallel-execution race conditions
[Theory]
[InlineData("vektis_no_year.csv")]   // no year in filename → returns empty
[InlineData("vektis_empty.csv")]     // empty file → returns empty
public void Parser_ParseFile_EdgeCases_ReturnsEmpty(string fileName)
{
    var tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
    // ...
    Assert.Empty(VektisCsvParser.ParseFile(path));
}

Test count: 22 → 25.

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 file contains 3 [Fact] methods across 110 lines and includes a placeholder test, overly verbose integration-style assertions, and missing coverage for VektisCsvParser edge cases.

Current State

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

Test Quality Analysis

Strengths ✅

  • Parser_CanParseRealVektisFile runs the actual parser against real CSV data and validates column names, counts, and decimal parsing — good end-to-end confidence
  • Service_CanLoadSampleCsv_FromConfiguration validates DI wiring, multi-year loading, and a minimum record count in a single test
  • Uses Assert.NotNull(dir) and Assert.NotEmpty(...) instead of raw boolean checks in key places

Areas for Improvement 🎯

1. Remove placeholder Assert.True(true) test

Current:

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

Recommended: Delete Truth_IsTrue entirely. It adds noise to the test suite and a permanently-green test gives false confidence.

Why this matters: Placeholder tests inflate pass counts, hide coverage gaps, and confuse future contributors about intent.


2. Decompose the overlong Parser_CanParseRealVektisFile test

The test is 44 lines long and manually re-implements CSV parsing logic before calling VektisCsvParser.ParseFile. The manual pre-validation and the parser call should live in separate, focused tests.

View current test (condensed)
[Fact]
public void Parser_CanParseRealVektisFile()
{
    // 20 lines: locate the file, open it, read the header, split it, assert column names...
    // Then 10 more lines asserting decimal parsing...
    // Then finally: call VektisCsvParser.ParseFile and assert > 0 records
}

Recommended decomposition:

// ✅ Test 1 – focused on column contract
[Fact]
public void Parser_CsvHeaderHasExpectedColumns()
{
    var targetFile = FindVektisCsvFile();
    using var reader = new StreamReader(targetFile);
    var headers = reader.ReadLine()!.Split(';')
        .Select(h => h.Trim().ToLowerInvariant()).ToArray();

    Assert.Equal(31, headers.Length);
    Assert.Equal("geslacht", headers[0]);
    Assert.Equal("aantal_verzekerdejaren", headers[4]);
}

// ✅ Test 2 – focused on parser output
[Fact]
public void Parser_ParseFile_ReturnsNonEmptyRecords()
{
    var targetFile = FindVektisCsvFile();

    var records = VektisCsvParser.ParseFile(targetFile);

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

// shared helper
private static string FindVektisCsvFile()
{
    var dir = new DirectoryInfo(AppContext.BaseDirectory);
    while (dir != null && !File.Exists(Path.Combine(dir.FullName, "CareMetrics.slnx")))
        dir = dir.Parent;
    var csvDir = Path.Combine(dir!.FullName, "CareMetrics.API", "Data", "vektis", "postcode3");
    var files = Directory.GetFiles(csvDir, "*.csv");
    return files.FirstOrDefault(f => f.Contains("2023")) ?? files[0];
}

Why this matters: A single failing assertion in a 44-line test makes it hard to pinpoint the root cause. Smaller tests fail faster and point directly to the broken contract.


3. Add [Theory] / [InlineData] for VektisCsvParser edge cases

Currently zero parameterised tests exist in this file. Several edge-case scenarios for ParseFile / ParseDirectory should be parameterised:

// ✅ Edge cases for ParseFile
[Theory]
[InlineData("vektis_no_year.csv")]        // no year in filename → returns empty
[InlineData("vektis_empty.csv")]          // empty file → returns empty
public void Parser_ParseFile_EdgeCases_ReturnsEmpty(string fileName)
{
    var tempDir = Path.Combine(Path.GetTempPath(), "vektis_test");
    Directory.CreateDirectory(tempDir);
    var path = Path.Combine(tempDir, fileName);
    File.WriteAllText(path, "");   // empty content for both cases

    var result = VektisCsvParser.ParseFile(path);

    Assert.Empty(result);

    Directory.Delete(tempDir, true);
}

// ✅ ParseDirectory with empty directory returns empty list
[Fact]
public void Parser_ParseDirectory_EmptyDirectory_ReturnsEmpty()
{
    var tempDir = Path.Combine(Path.GetTempPath(), "vektis_empty_dir_test");
    Directory.CreateDirectory(tempDir);

    var result = VektisCsvParser.ParseDirectory(tempDir);

    Assert.Empty(res...

</details>



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

- Fixes rowantervelde/agentic-development#37

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

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs.

… tests

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