As the Speaker Meet application progressed, a new requirement was introduced. Speakers had to be approved before they were visible in parts of the system. This included the full listing of speakers, returning of speaker detail information, and through search results.

In this scenario, a developer came in to help out with the implementation. This developer was not familiar with TDD and did not write tests to validate his work. The new requirement was implemented and a code review was submitted:

public Models.SpeakerDetail Get(int id)
{
  var speaker = _repository.Get(id);

  if (speaker == null || speaker.IsDeleted || speaker.IsActive)
  {
    throw new SpeakerNotFoundException(id);
  }

  var gravatar = _gravatarService.GetGravatar(speaker.EmailAddress);

  return new Models.SpeakerDetail
  {
    Id = speaker.Id,
    Name = speaker.Name,
    Location = speaker.Location,
    Gravatar = gravatar
  };
}

And a change to the class was added:

public class Speaker
{
  public int Id { get; set; }

  [Required]
  [StringLength(50)]
  public string Name { get; set; }

  [Required]
  [StringLength(50)]
  public string Location { get; set; }

  [Required]
  [StringLength(255)]
  public string EmailAddress { get; set; }
  
  public bool IsDeleted { get; set; }
  
  public bool IsActive { get; set; }
}

Can you spot the issue?

The code was reviewed and comments left. However, the comments were misunderstood (or just flatly ignored) and the code was committed, merged, and pushed through the deployment process. A breakdown for sure, but one that happens from time to time.

The CI server ran the test suite. The existing tests passed. The bug was not discovered, as there was no existing scenario that would have caught the error. Since new tests were not created, there was no test failure. The CD process ran and the code made it into production.

So what test can be added to ensure the proper code is implemented? When dealing with bugs, it is often best to simply write the test that verifies the incorrect behavior. In this case, we want an error to be thrown. So, the below test should assert that the correct error is thrown:

[Fact]
public void GivenSpeakerIsNotActiveThenSpeakerNotFoundException()
{
  // Arrange
  var expectedSpeaker = SpeakerFactory.Create(_fakeRepository);
  expectedSpeaker.IsActive = false;
  var service = new SpeakerService(_fakeRepository, _fakeGravatarService);
  // Act
  var exception = Record.Exception(() => service.Get(expectedSpeaker.Id));
  // Assert
  Assert.IsAssignableFrom<SpeakerNotFoundException>(exception);
}

Make this new test pass by modifying the service:

public Models.SpeakerDetail Get(int id)
{
  var speaker = _repository.Get(id);

  if (speaker == null || speaker.IsDeleted || !speaker.IsActive)
  {
    throw new SpeakerNotFoundException(id);
  }

  var gravatar = _gravatarService.GetGravatar(speaker.EmailAddress);

  return new Models.SpeakerDetail
  {
    Id = speaker.Id,
    Name = speaker.Name,
    Location = speaker.Location,
    Gravatar = gravatar
  };
}

However, with this change, a number of existing tests will now break. This is because the default value for the IsActive property is false.

To quickly get these tests to pass, you could do something like:

public bool IsActive { get; set; } = true;

This could potentially introduce unexpected results, so be sure to create some guard tests to verify correctness.

This explains why this bug wasn't initially caught. The IsActive property was added to the database with a default value of true. The bug wasn't discovered until new speakers were added to the database with a value of false in the IsActive column. Once the incorrect behavior was discovered, the defect was easily identified and remedied.