Test A requires reviewing other files to prove it is correct. Test B doesn’t but has more than one assert

Question

Say you want to test an update capability across a CRUD api.

Test A updates a field, queries it, and asserts that it now has the value from the update. Knowledge about the starting value (and that it differs from the test value) comes from the way the initial data for the field is prepared: it is done by an utility component which initializes the test data, and the test author can determine its value by looking into the source code file of the utility when writing the test. This review should prevent creating a test that, for example, updates the value from "TESTDATA" to "TESTDATA".

Test B first queries the value at the start - directly from the field under test - and asserts that it’s not the same as the test value before doing and asserting the update.

Both tests confirm the same update behavior. A requires reviewing another file. B requires an extra assert.

Is one preferable? Why?

Argument for A

Code reviews will catch if the values are the same.

Argument for B

Good automated tests minimize what you need to read.

Answer 1

Relying on some value which was coded in a different file (a reusable utility class somewhere else in the codebase) bears the risk the value will be changed by someone else after the code review happened. It is quite unlikely the person who changes the file will remember that months or years ago in another part of the code base a test exists which relies indirectly on the original value. In an unfortunate case, the "default value from that other file" becomes equal to the "update value" afterwards, which makes Test A pointless.

So this clearly shows that the approach for Test A is flawed, it ignores that the code base can evolve and introduces a hidden dependency. It is a kind of DRY violation, since the test unnessarily duplicates the knowledge of what value was stored in "that other file".

Test B is IMHO fine, even if it uses two asserts - which I am ok with. Applying a rule like "strictly one assert per test" is IMHO dogmatic nonsense. When one follows the "arrange-act-assert" pattern, it can be perfectly valid to make a test more robust by having an extra "assert" statement for the "arrange" step, or multiple assert statements in the "assert" step itself for testing the outcome of a single operation. See also Is it OK to have multiple asserts in a single unit test?.

Still I would prefer Test C instead:

• query the original value x from the field

• modify x in some simple way (when it is an integer, add 1, when it is a string, append or change a character, when it is a boolean, invert it etc.). Lets call the new value y

• update the field to y

• assert that the field has the new value y

This way, the test does neither have to rely on some code review from the past, nor has to use two asserts. It just verifies the update behaviour and avoids to make any assumptions about prior values of the field.

If one has concerns about modifying a potentially unknown value, I would probably add a simple preparation first: pick a fixed, hardcoded value x and set the field to x first. Here, one needs to use an extra assert to make sure the field really has got the initial value x before it is changed to y. And I definitely don't see the two asserts as a downside here.

Let me add after I wrote all of the above, a simpler solution for Test C was suggested by @IMil.

Answer 2

The core question here seems to be whether you should test your test arrangement. In other words, who tests the testers?

This is highly subjective, and the answer is based on what you get out of it. You've pointed at a trivially assertable check, but it won't always be this easy. There will be a subset of tests where asserting the arrangement will become a disproportionate amount of the effort required.

If the update fails we can't detect the failure if the update was supposed to change "TESTDATA" to "TESTDATA".

The core issue I see here is a mistrust of how the initial state was arranged. If you mistrust your test designers to that degree; I am really concerned about how they're designing the rest of the tests as well, and you cannot cover that just by slapping on some more assertions.

The only way your bug would slip through is if (at the same time) the developer failed to have the update logic implemented and the tester did not arrange the test with correctly assertable values, but they still knew to write this test that is otherwise complete. That is an oddly specific scenario to try and catch.

So is it bad that you assert this? At face value, no. But the reason for doing so seems to be part of a larger problem (not trusting who wrote the test), and you're only patching one part (arranging the test data) and not the others (overall test design, assertion logic, ...); which is making me feel that this is a well hidden XY problem.

However, there are edge cases where you'd like to test your data and it's not a matter of developer mistrust, for example because you're using real world data or because you generated data because it was too much to handcraft it. In those cases, you can justify testing the data.
But I maintain that you shouldn't be adding those assertions to the same test. You should be testing the quality of the data separately from testing the behavior of your application, as one has nothing to do with the other. Just because I provide bad test data does not mean the app is broken, and vice versa.

Answer 3

If I understand correctly, the only problem with test A is that the initial value of the field might happen to be exactly what the test is trying to store there.

Test B indeed fixes this problem. But there's a simpler option.

Just have a parameterised test with a list of test cases. E.g. (in pseudocode) instead of

TestA() {
    field.SetValue("New Value");
    Assert.AreEqual(field.getValue(), "NewValue");
}

you should have

[TestCases("New value", "Even newer value", "Completely new value")]
TestC(value) {
    field.SetValue(value);
    Assert.AreEqual(field.getValue(), value);
}

In this case it's obvious that even if the original value accidentally matches one test case, in other cases the value will be new.

Answer 4

Test B is preferable because the interface for the object under test is tested, rather than the internal implementation details. It will continue to work correctly when you change the internal implementation to use a different format, or whatever.

Test A requires a knowledge of the implementation. This can lead to you writing a duplicate of the class under test in your test project.

If you are writing tests for a repository for example, and use strategy A, your database setup and tear down code for the various test cases can easily exceed the complexity of the repository itself. A column added to a table can break all your tests.

PS. I would update to a random value to avoid the pre check. But get value, update value, get value again, assert its changed as expected is a common pattern.

Answer 5

Why do you need to know that the starting value is different than the test value? This immediately makes me suspicious that we are either:

• Missing a requirement that deserves its own test.

• OR the test is executing in an environment that results in non-deterministic tests.

If the starting value is not a missing requirement, and tests are deterministic, then I would argue the starting value is not important. Test A is fine. Why do reviewers need to see this other file? That other file is setup code, which should be unnecessary to read in order to understand the test case.

As a thought experiment, consider how much effort would be required to debug a test failure in both cases.

If Test A fails, how important is the starting value? Is knowing the starting value essential to finding the root cause of a test failure? Do you not have the ability to step through this unit test with an IDE so you can analyze the initial value?

For Test B, why on Earth would the first assert ever fail? I have written tests before where an assert is part of the setup — but only as a debugging tool for wonky or unstable tests. As soon as I root out the instability, I usually remove the assert in the test setup.

To me, asserting the initial value means that I, as the test author, trust the initial state of the system under test as much as I trust the system to change the state correctly (e.g. "I don't trust the initial state or the system under test"). The first assert in Test B feels funny, because I should at least trust the initial state of the system.

But funny feelings are not a good reason to remove an assert. It might be the reason to add that first assert in Test B.

Whichever you chose, go with the strategy that makes your life easier. Tests shouldn't pass. Tests should fail correctly, and if you are doing that the test is fine however it is written. But still, the first assert in Test B bugs me. Why do we need it? That's the thing to figure out. If you don't need it, get rid of it. If you do need it, then consider writing a test to associate the initial value with some requirement. If there is no requirement, but asserting the initial value is deemed necessary, add a comment above the assert stating why this was needed, and describe the conditions which should prompt its removal.

Answer 6

Of the two approaches as written, I think approach B is closer to correct. At a high level, the purpose of setup code is to ensure the SUT† is in the correct state for the test. We normally achieve that through initializing values, setting up mocks, creating test files, frobnicating widgets, and the like, but asserting something is also a way of ensuring it. I don't consider complaints about multiple assertions valid here; to me, an assertion about the integrity of the test is fundamentally different to an assertion about the SUT itself.

That said, I think approach A can be made correct by making the helper code configurable. If the helper code picks an initial value, then the test cannot be validated by reading it; the helper and the test might pick the same initial and updated value, respectively. If the test picks both values, then reading the test makes it obvious whether they're the same or not. This approach has the added benefit of being generally applicable; if your REST API provides a means to retrieve data, testing it likely runs into the opposite problem (the helper and the test need to pick the same value), which can also be avoided in the same way.

// Approach A as written

@Test
void testGet() {
  commonHelper.setupInitialValue();
  var actual = makeGetRequest();
  assertThat(actual).isEqualTo("initial value");
}

@Test
void testPut() {
  commonHelper.setupInitialValue();
  makePutRequest("updated value");
  var actual = commonHelper.getCurrentValue();
  assertThat(actual).isEqualTo("updated value");
}

// Modified Approach A with configuration

@Test
void testGet() {
  commonHelper.setupInitialValue("initial value");
  var actual = makeGetRequest();
  assertThat(actual).isEqualTo("initial value");
}

@Test
void testPut() {
  commonHelper.setupInitialValue("initial value");
  makePutRequest("updated value");
  var actual = commonHelper.getCurrentValue();
  assertThat(actual).isEqualTo("updated value");
}

Of course, this approach still permits the possibility of a bug in the helper code. Where to draw the line there is a highly contextual decision call; since this question doesn't make sense in a context where the trade-off isn't (deemed) worth it, I believe that to be out of scope.

† System Under Test

Answer 7

If your crud code is automatically generated you should not be testing the crud functionality at all. Unless you have a reason to distrust the underlying Generator functionality there is no need to spend cycles here.

If you need a resilient smoke test, generate a random value and then use the check and set procedure (the second version). Note that this version is twice expensive on the underlying code.

There is no reason to just have one assert in a test. If this was the case then every test would be a single function which it isn't by design. A test may also want to call out several different factors that it wants to show distinctively. For this multiple asserts are necessary.

The important factor is the naming and intent of the test. If the name of the test cannot clearly state what it's purpose is, then it is not a good test.

A test should not have a single assert, but it should have a single purpose.