TDD Pitfalls — Common Mistakes and How to Avoid Them

TDD fails for one of these reasons:
  1. Tests test implementation, not behaviour — they break on every refactor
  2. Over-mocking — tests pass but the real system doesn't work
  3. Test suite is too slow — developers stop running it
  4. Tests give false confidence — high coverage, bugs still ship
  5. Tests are written after the code — defeats the design benefit of TDD

These are not arguments against TDD.
They are arguments against doing TDD badly.

// BAD: test that verifies private method calls and internal state
[Fact]
public void ApprovePrescription_CallsValidateInrFirst()
{
    // Testing that ValidateInr() is called — not that the prescription is approved
    var service = new PrescriptionApprovalService();
    service.ApprovePrescription(prescriptionId, 2.5m, DateTime.UtcNow);

    // This test breaks every time you rename ValidateInr() or extract it
    // even if behaviour is identical
    service.ValidateInrCallCount.Should().Be(1); // internal counter — fragile
}

// GOOD: test observable behaviour, not internal mechanics
[Fact]
public void ApprovePrescription_WithValidInr_ReturnsSuccess()
{
    var prescription = Prescription.CreateDraft(...);
    var result = prescription.Approve(2.5m, DateTime.UtcNow, Guid.NewGuid());

    // Tests what matters: the outcome, not how it was achieved
    result.IsSuccess.Should().BeTrue();
    prescription.Status.Should().Be(PrescriptionStatus.Approved);
}

// BAD: mocking everything, including the system under test's own collaborators
// This test passes even if the real integration is completely broken

[Fact]
public void CreatePrescription_SavesToRepository()
{
    var repo      = Substitute.For<IPrescriptionRepository>();
    var validator = Substitute.For<IPrescriptionValidator>();
    var notifier  = Substitute.For<INotificationService>();
    var clock     = Substitute.For<IClock>();

    validator.Validate(Arg.Any<CreatePrescriptionCommand>())
        .Returns(ValidationResult.Success);

    var handler = new CreatePrescriptionHandler(repo, validator, notifier, clock);
    await handler.Handle(command, CancellationToken.None);

    await repo.Received(1).AddAsync(Arg.Any<Prescription>(), Arg.Any<CancellationToken>());
}

// What's wrong:
// → Validator is mocked — real validation never runs
// → Clock is mocked — real DateTime.UtcNow never involved
// → The test proves the handler CALLS repo.AddAsync, not that a prescription is created correctly
// This test passes even if the handler creates a completely wrong Prescription object

// BETTER: use real collaborators where they're fast and deterministic
[Fact]
public async Task CreatePrescription_WithValidData_PrescriptionHasCorrectState()
{
    var repo = new InMemoryPrescriptionRepository();  // real in-memory impl
    var handler = new CreatePrescriptionHandler(repo);
    var command = new CreatePrescriptionCommand(patientId, "Warfarin", 5m, "mg");

    await handler.Handle(command, CancellationToken.None);

    var saved = repo.All.Single();
    saved.MedicationName.Value.Should().Be("Warfarin");
    saved.Status.Should().Be(PrescriptionStatus.Draft);
}

Symptoms:
  → `dotnet test` takes more than 60 seconds for unit tests
  → Developers stop running tests between commits
  → "I'll run tests before pushing" — often forgotten

Causes:
  → Unit tests hitting a real database (not InMemory or Testcontainers)
  → Thread.Sleep() or Task.Delay() in tests
  → Tests instantiating the full DI container when not needed
  → Tests loading appsettings.json and all configuration providers

Fixes:
  → Unit tests: no I/O, no network, no filesystem — pure in-memory
  → Integration tests: Testcontainers (one container, shared across the test class)
  → Separate test projects: ClinicalSystem.UnitTests vs ClinicalSystem.IntegrationTests
  → CI runs unit tests on every commit, integration tests nightly or on PR

Target times:
  → Unit test suite (all domain logic): under 10 seconds
  → Integration tests (real DB): under 3 minutes

// BAD: testing that ASP.NET Core routing works, or that EF Core can save an entity
[Fact]
public async Task AddPatient_StoresInDatabase()
{
    var options = new DbContextOptionsBuilder<PatientsDbContext>()
        .UseInMemoryDatabase("test")
        .Build();

    using var context = new PatientsDbContext(options);
    var patient = new Patient { Id = Guid.NewGuid(), FullName = "Jane Doe" };
    context.Patients.Add(patient);
    await context.SaveChangesAsync();

    // This tests EF Core, not your code
    var found = await context.Patients.FindAsync(patient.Id);
    found.Should().NotBeNull();
}

// You are testing EF Core's Add/Find — that's Microsoft's job, not yours.
// Your code doesn't have any business logic in this test.

// BETTER: test your business logic
[Fact]
public void Patient_Create_WithValidMrn_Succeeds()
{
    var result = Patient.Create(
        PatientMrn.Of("MRN001"),
        FullName.Of("Jane", "Doe"));

    result.IsSuccess.Should().BeTrue();
    result.Value.Mrn.Value.Should().Be("MRN001");
}

// BAD: assertion that can never fail
[Fact]
public void GetPatients_ReturnsResult()
{
    var service = new PatientService(Substitute.For<IPatientRepository>());
    var result  = service.GetAllAsync().Result;

    // This assertion is always true — even if result is an empty list
    // or null, Should().NotBeNull() catches nothing meaningful
    result.Should().NotBeNull();
}

// A test that can never fail provides false confidence.
// It inflates coverage metrics while catching no real bugs.

// BETTER: test the specific outcome
[Fact]
public async Task GetPatients_WithActivePatients_ReturnsOnlyActiveOnes()
{
    var repo = Substitute.For<IPatientRepository>();
    repo.GetAllAsync(Arg.Any<CancellationToken>())
        .Returns(new[]
        {
            Patient.Create(PatientMrn.Of("MRN001"), FullName.Of("Jane", "Doe")).Value,
            Patient.Create(PatientMrn.Of("MRN002"), FullName.Of("John", "Smith")).Value
        });

    var service = new PatientService(repo);
    var result  = await service.GetActiveAsync(CancellationToken.None);

    result.Should().HaveCount(2);
    result.All(p => p.IsActive).Should().BeTrue();
}

Pitfall                               → Fix
──────────────────────────────────────────────────────────────────────────
Testing private methods               → Test public behaviour only
Mocking value objects                 → Use real value objects (they're fast)
One giant Arrange block               → Use Builder pattern or factory methods
Test names: "Test1", "Method_Works"   → Describe the scenario and expected outcome
Catching and swallowing exceptions    → Let exceptions propagate; assert on thrown
Assert first (result not null) only   → Assert the specific value or state
No test isolation (shared static state)→ Each test creates its own instances
Thread.Sleep in tests                 → Use virtual clock (IClock) or ManualResetEvent