Tautological assertion by Mark Seemann
It's surprisingly easy to write a unit test assertion that never fails.
Recently I was mob programming with a pair of IDQ's programmers. We were starting a new code base, using test-driven development (TDD). This was the first test we wrote:
[Fact] public async Task HandleObserveUnitStatusStartsSaga() { var subscribers = new List<Guid> { Guid.Parse("{4D093799-9CCC-4135-8CB3-8661985A5853}") }; var sut = new StatusPolicy { Data = new StatusPolicyData { UnitId = 123, Subscribers = subscribers } }; var subscriber = Guid.Parse("{003C5527-7747-4C7A-980E-67040DB738C3}"); var message = new ObserveUnitStatus(123, subscriber); var context = new TestableMessageHandlerContext(); await sut.Handle(message, context); Assert.Contains(subscriber, sut.Data.Subscribers); }
This unit test uses xUnit.net 2.4.0 and NServiceBus 7.1.10 on .NET Core 2.2. The System Under Test (SUT) is intended to be an NServiceBus Saga that monitors a resource for status changes. If a unit changes status, the Saga will alert its subscribers.
The test verifies that when a new subscriber wishes to observe a unit, then its ID is added to the policy's list of subscribers.
The test induced us to implement Handle like this:
public Task Handle(ObserveUnitStatus message, IMessageHandlerContext context) { Data.Subscribers.Add(message.SubscriberId); return Task.CompletedTask; }
Following the red-green-refactor cycle of TDD, this seemed an appropriate implementation.
Enter the Devil #
I often use the Devil's advocate technique to figure out what to do next, so I made this change to the Handle method:
public Task Handle(ObserveUnitStatus message, IMessageHandlerContext context) { Data.Subscribers.Clear(); Data.Subscribers.Add(message.SubscriberId); return Task.CompletedTask; }
The change is that the method first deletes all existing subscribers. This is obviously wrong, but it passes all tests. That's no surprise, since I intentionally introduced the change to make us improve the test.
False negative #
We had to write a new test, or improve the existing test, so that the defect I just introduced would be caught. I suggested an improvement to the existing test:
[Fact] public async Task HandleObserveUnitStatusStartsSaga() { var subscribers = new List<Guid> { Guid.Parse("{4D093799-9CCC-4135-8CB3-8661985A5853}") }; var sut = new StatusPolicy { Data = new StatusPolicyData { UnitId = 123, Subscribers = subscribers } }; var subscriber = Guid.Parse("{003C5527-7747-4C7A-980E-67040DB738C3}"); var message = new ObserveUnitStatus(123, subscriber); var context = new TestableMessageHandlerContext(); await sut.Handle(message, context); Assert.Contains(subscriber, sut.Data.Subscribers); Assert.Superset( expectedSubset: new HashSet<Guid>(subscribers), actual: new HashSet<Guid>(sut.Data.Subscribers)); }
The only change is the addition of the last assertion.
Smugly I asked the keyboard driver to run the tests, anticipating that it would now fail.
It passed.
We'd just managed to write a false negative. Even though there's a defect in the code, the test still passes. I was nonplussed. None of us expected the test to pass, yet it does.
It took us a minute to figure out what was wrong. Before you read on, try to figure it out for yourself. Perhaps it's immediately clear to you, but it took three people with decades of programming experience a few minutes to spot the problem.
Aliasing #
The problem is aliasing. While named differently, subscribers and sut.Data.Subscribers is the same object. Of course one is a subset of the other, since a set is considered to be a subset of itself.
The assertion is tautological. It can never fail.
Fixing the problem #
It's surprisingly easy to write tautological assertions when working with mutable state. This regularly happens to me, perhaps a few times a month. Once you've realised that this has happened, however, it's easy to address.
subscribers shouldn't change during the test, so make it immutable.
[Fact] public async Task HandleObserveUnitStatusStartsSaga() { IEnumerable<Guid> subscribers = new[] { Guid.Parse("{4D093799-9CCC-4135-8CB3-8661985A5853}") }; var sut = new StatusPolicy { Data = new StatusPolicyData { UnitId = 123, Subscribers = subscribers.ToList() } }; var subscriber = Guid.Parse("{003C5527-7747-4C7A-980E-67040DB738C3}"); var message = new ObserveUnitStatus(123, subscriber); var context = new TestableMessageHandlerContext(); await sut.Handle(message, context); Assert.Contains(subscriber, sut.Data.Subscribers); Assert.Superset( expectedSubset: new HashSet<Guid>(subscribers), actual: new HashSet<Guid>(sut.Data.Subscribers)); }
An array strictly isn't immutable, but declaring it as IEnumerable<Guid> hides the mutation capabilities. The test now has to copy subscribers to a list before assigning it to the policy's data. This anti-aliases subscribers from sut.Data.Subscribers, and causes the test to fail. After all, there's a defect in the Handle method.
You now have to remove the offending line:
public Task Handle(ObserveUnitStatus message, IMessageHandlerContext context) { Data.Subscribers.Add(message.SubscriberId); return Task.CompletedTask; }
This makes the test pass.
Summary #
This article shows an example where I was surprised by aliasing. An assertion that I thought would fail turned out to be a false negative.
You can easily make the mistake of writing a test that always passes. If you haven't tried it, you may think that you're too smart to do that, but it regularly happens to me. Other TDD practitioners have told me that it also happens to them.
This is the reason that the red-green-refactor process encourages you to run each new test and see it fail. If you haven't seen it fail, you don't know if you've avoided a tautology.
