6

Assume I have the following two functions:

function storeObject(object) {
    // Connect to database
    // Prepare query
    // Execute query
}

function retrieveObjectWith(id) {
    // Connect to database
    // Prepare query
    // Execute query
    // Parse results

    return object;
}

And that I want to write tests to them:

function testStore() {
    storeObject(storedObject)

    // Connect to mocked database
    // Prepare query to retrieve stored object
    // Execute query and retrieve stored object

    [retrievedObject] should equal [storedObject]
}

function testRetrieve() {
    // Connect to mocked database
    // Prepare query to store object
    // Execute query and store object

    retrievedObject(storedObject)

    [retrievedObject] should equal [storedObject]
}

I could simplify the second test if I "trusted" the results from the first, like this:

function testRetrieve() {
    // Since I have a separate test for storeObject, I can use it here
    storeObject(testObject)
    retrievedObject = retrieveObjectWithId(testObject.id)

    [retrievedObject] should equal [testObject]
}

Question is

Are there any cons of using the storeObject(object) instead of rewriting its logic inside the test?

Is it considered a bad practice to do this?

appa yip yip
  • 425
  • 6
  • 17
  • 1
    Does this answer your question? [How to write unit tests a method with a result that is highly based on another method](https://softwareengineering.stackexchange.com/questions/389039/how-to-write-unit-tests-a-method-with-a-result-that-is-highly-based-on-another-m) – gnat May 20 '20 at 21:33
  • @gnat I don't think so. From what I understand, that question is more about testing a function whose result depends on another function. In this case, one function does not call the other, but one could be used to test the result of the other. – appa yip yip May 20 '20 at 21:38
  • @appayipyip I'd ask you to reconsider the accepted answer. You can find a lot of material ou there supporting the case for testing `retrieveObjectWithId` by calling `storeObject`, because this means that your tests know only the external API of the object(s) being tested, and thus will be much more resistant to structure-only changes as the code matures/evolves. A simple example from Kent Beck's: github.com/KentBeck/TDD-Tyrant/blob/master/src/TyrantTest.java. It is a test for a simple binary DB utility, and he tests the `get` operation by doing a `put`. – MichelHenrich May 21 '20 at 12:58
  • Consider if you want to be able to execute your tests in any order, and in parallel. This will make your test suite the most robust. – Thorbjørn Ravn Andersen May 21 '20 at 14:57
  • @appayipyip: The question gnat linked uses a return value and you're using an indirect outcome (i.e. the existence of this object in the storage), but the principle is the same on how to approach your testing strategy. – Flater May 21 '20 at 20:36

2 Answers2

10

What is your system under test? Quite often, testing a method in isolation is not sensible. Often, the value of a system is provided through the interplay of multiple methods. Then, when writing a test that verifies that the value is being provided by the system, calling multiple methods is perfectly fine.

Your test examples hint at three possible kinds of value the system may provide. First, there are two behaviours regarding how the database is used:

  • Scenario: writes to the database use a particular format
    Given a database
    When I storeObject({ id: 123, name: "foo", ... })
    Then the database table objects contains a row (123, "foo", ...)

  • Scenario: reading from the database understands a particular format
    Given a database with object (789, "bar", ...)
    When I retrieveObject(789)
    Then the object is { id: 789, name: "bar", ...}

These tests are likely to be integration tests since they verify how the system interacts with a particular database schema. Such tests have a lot of value when other systems use the same database, or when your system has to support multiple database schemas.

But for a unit test, we are probably interested in a completely different property:

  • Scenario: can retrieve stored objects
    Given a database
    And some object with an id
    When I storeObject(object)
    And I retrieveObject(id)
    Then I get the same object back

How does the store/retrieve work internally? Doesn't matter, as long as the two functions are compatible. What specific object am I storing? Doesn't matter, as long as I get the same object back (→ consider test parameterization).

This third test should definitely be present because it likely describes the value your system is supposed to supply. Testing implementation details like the specific data formats used can aid debuggability of your system, but also makes your tests more brittle. If you change your system to use a different data format you might have to update the first two tests, even though the system continues to provide its main value per the third test.

Mantras like “every test should only test one thing” are useful rules of thumb, but only if you think about what “one thing” is in this context. In particular, consider what the system under test is (function? class? module? service?) and what value this system should provide. Then, write a test that verifies that the system provides this value, without focusing on how the system provides this value.

amon
  • 132,749
  • 27
  • 279
  • 375
  • 1
    All very well, but as the SUT ends up being a complex system we are no longer talking about a unit test. That would be an integration test. – Martin Maat May 21 '20 at 10:27
  • I agree with Martin here. You're not wrong about the three tests all being useful, but the third one is an integration test, not a unit test. For a **unit** test, the object should be in the data storage after the "arrange" part of the test (regardless of how it's done, but ideally via direct access to a mocked storage object), the "act" part of the test _only_ focuses on the `retrieveObject` call, and the assert only fails based on `retrieveObject`'s result. That is a unit test. An integration test can chain them, and is a meaningful additional test but does not replace the unit test's value. – Flater May 21 '20 at 20:41
4

Well, no. A unit test is supposed to test one thing. It can be one particular way of running a method, or it can be an entire method if it's simple enough for that, but it certainly isn't for testing multiple methods. Therefore, one test assuming the validity of a different method is standard operating procedure. If there were a bug in the other method, it would show up in the other unit test. (This is why code coverage is important in testing.)

Kilian Foth
  • 107,706
  • 45
  • 295
  • 310
  • 1
    I read your 'well, no' as 'no, there are no cons of using `storeObject(object)` in second test'. Actually, there is **at least one objection**: if `storeObject` failed then you got two (many) broken tests. It could lead to many (minor) issues like bad reports or several people which would solve problems (in worst case - one developer for each fallen test) – ADS May 21 '20 at 01:11
  • That’s why you hire developers, not monkeys. Either you have broken tests because development isn’t finished, then you fix your code and the errors disappear one after the other. Or someone broke things, then you have _one_ person fixing them. – gnasher729 May 21 '20 at 07:31
  • @ADS I'd like to support your comment by adding that for it's important for unit tests to be "reliable" - when they fail, they should do so because the scenario being tested has a fault and NOT as a side-effect of some other implementation detail failing – Stewart Ritchie May 21 '20 at 10:55
  • -1 due to this being simply incorrect. Kent Beck, Robert C. Martin (Uncle Bob), Martin Fowler and various other authoritative authors on the concept of (automated) unit testing would vehemently disagree with this answer. Unit testing is not about testing a single method at a time or even a single class. It is entirely OK to use the second approach, even preferable, as it is less coupled to the implementation details of the object under test, touching only its exposed API. Example here: https://github.com/KentBeck/TDD-Tyrant/blob/master/src/TyrantTest.java (lines 16-17). – MichelHenrich May 21 '20 at 12:49
  • @MichelHenrich There's usefulness in distinguishing between automated tests that exercise a system end-to-end, which is what the Kent Beck link you provided does, and an automated test that exercises a single piece of functionality. When I hear the term 'unit test', I expect a test that exercises a single piece of functionality. I would describe the Kent Beck example as an integration test. Not everybody uses these terms in exactly the same way, but that doesn't mean this answer is incorrect. It's just using a particular definition of 'unit test'. TDD is not the same thing as unit testing. – Eric King May 21 '20 at 15:12