27

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.

  • 2
    Think from another angle: Do you want to have tests which not failed when you change logic in the system under the test? – Fabio Dec 25 '18 at 10:17
78

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.

  • @JaquesB, then it would not be a unit test? or would it? I believe it would be an integration test if the database was involved. Is that right? Are you saying don't use unit tests if the business logic changes a lot? – w0051977 Dec 24 '18 at 12:33
  • @w0051977: Depends how you write the test. If the test includes actually changing changing something in a database, then it would be an integration test. – JacquesB Dec 24 '18 at 12:39
  • 3
    @w0051977 better idea - don't have the repository be a dependency of the component responsible for implementing business rules. Have a higher-level orchestration that calls the repository and then invokes the business rules. Now you can unit test the business rules in isolation. – Ant P Dec 24 '18 at 14:14
  • 5
    @w0051977 of course it is - tests specify behaviour. If the rules governing the behaviour of a component change, then the tests must change to reflect the change in behaviour. What shouldn't need to change are tests that specify behaviours other than what is changing. If the behaviour is determined by the database, then a test covering some other code is inherently unrelated and should not need to change unless that database logic is within the scope of the test. That scope is for you to define and the semantics of whether it's a unit test or integration test are not really important. – Ant P Dec 24 '18 at 15:11
  • 3
    @w0051977 extending this idea somewhat, if business logic changes or a bug is fixed and the tests don't need adjusting, it's a sign that the tests aren't covering sufficient cases and should generally be expanded. – Morgen Dec 24 '18 at 18:17
14

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.

  • I tool the freedom to improve some minor details in your answer. Please check if I got your intentions right. – Doc Brown Dec 24 '18 at 17:07
  • There is no indication in the original post that a database is involved. So claiming that it assumes that Offer1 is already in the database is bizarre. – Winston Ewert Dec 24 '18 at 17:13
  • 2
    @WinstonEwert: there is a clear indication, you have to read the question more carefully. I did not realize it at the first read, too, but it is indeed what the OP is talking of. – Doc Brown Dec 24 '18 at 17:21
  • @mdfst13, I'm sorry I missed that. However, what the OP is saying is that the conditions are sometimes in a database and sometimes in the domain model. Your answer is perfectly fine for the first case, and irreverently in the second. If you'll edit your answer to clarify that point, I'll remove my hasty downvote. – Winston Ewert Dec 25 '18 at 0:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.