Is a Unit test considered brittle if it fails when the business logic changes?

Question

Please see the code below; it tests to see if a person with Gender of female is eligible for offer1:

[Fact]
public void ReturnsFalseWhenGivenAPersonWithAGenderOfFemale()
{
    var personId = Guid.NewGuid();
    var gender = "F";
    var person = new Person(personId, gender);

    var id = Guid.NewGuid();
    var offer1 = new Offer1(id,"Offer1");
    Assert.False(offer1.IsEligible(person));
}

This unit test succeeds. However, it will fail if 'Offer1' is offered to females in future.

Is it acceptable to say - if the business logic surrounding offer 1 changes then the unit test must change. Please note that in some cases (for some offers) the business logic is changed in the database like this:

update Offers set Gender='M' where offer=1;

and in some cases in the domain model like this:

if (Gender=Gender.Male)
{
  //do something
}

Please also note that in some cases the domain logic behind offers changes regularly and in some cases it does not.

Answer 1

This is not brittle in the usual sense. A unit test is considered brittle if it breaks due to implementation changes which does not affect the behavior under test. But if the business logic itself changes, then a test of this logic is supposed to break.

That said, if the business logic indeed changes often, perhaps it is not appropriate to hardcode the expectations into the unit tests. Instead you could test if the configurations in the database affects the offers as expected.

The name of the test Returns False When Given A Person With A Gender Of Female does not describe a business rule. A business rule would be something like Offers Applicable to M should not be applied to persons of gender F.

So you could write a test that confirms that if an offer is defined as only applicable to type M persons, then a type F person will not be indicated as eligible for it. This test will ensure the logic works even if the configuration of the specific offers change.

Answer 2

When the property is defined in the production database (or a clone for testing), this is not a unit test. A unit test checks a unit of work and does not require a particular external state to work. This assumes that Offer1 is defined in the database to be a male-only offer. That's external state. So this is more of an integration test, specifically a system or acceptance test. Note that acceptance tests are often not scripted (not run in a test framework but manually performed by human beings).

When the property is defined in the domain model with an if statement, the same test is a unit test. And it may be brittle. But the real problem is that the code is brittle. As a general rule, your code will be more resilient if business behavior is configurable rather than hard coded. Because a rush deployment to fix a small coding error should be rare. But a business requirement changing without notice is just a Tuesday (something that happens weekly).

You may be using a unit test framework to run the test. But unit test frameworks are not limited to running unit tests. They can and do run integration tests as well.

If you were writing a unit test, you would create both person and offer1 from the ground up with no reliance on database state. Something like

[Fact]
public void ReturnsFalseWhenGivenAPersonWithAGenderOfFemale()
{
    var personId = Guid.NewGuid();
    var gender = "F";
    var person = new Person(personId, gender);

    var id = Guid.NewGuid();
    var offer1 = new Offer1(id, "ReturnsFalseWhenGivenAPersonWithAGenderOfFemale");
    offer1.markLimitedToGender("M");

    Assert.False(offer1.IsEligible(person));
}

Note that this doesn't change based on the business logic. It's not asserting that offer1 rejects females. It is making offer1 the type of offer that rejects females.

You might create and configure the database as part of the test. In C#, using NUnit, or in Java's JUnit, you would set up the database in a Setup method. Presumably your test framework has a similar notion. In that method, you could insert records into the database with SQL.

If it is hard for you to write code that substitutes a test database for the production database, that sounds like a testing weakness in your application. For testing, it would be better to use something like dependency injection that allows for substitution. Then you could write tests that are independent of the current business rules.

A side benefit of this is that it is often easier for the business owner (not necessarily the corporate owner, more like the person responsible for this product in the corporate hierarchy) to configure the business rules directly. Because if you have this kind of technical framework, it is easy to allow the business owner to use a user interface (UI) to configure the offer. The business owner would select the limitation in the UI, and it would issue the markLimitedToGender("M") call. Then when the offer is persisted to the database, it would store this. But you wouldn't need to store the offer to use it. So your tests could create and configure an offer that doesn't exist in the database.

In your system as described, the business owner would have to put in a request to the technical group, which would issue the appropriate SQL and update the tests. Or the technical group has to edit your code and tests (or tests then code). That seems a rather heavyweight approach. You can do it. But your software (not just your testing) would be less brittle if you did not have to do so.

TL;DR: you can write tests like this, but you may be better off writing your software so you don't have to do so.