-1

Let's say that I have a class with a service that is going to be injected at runtime:

class Thing {

    private magic: IMagic; // Magically injected service

    public doStuff(){
        // Do a lot of stuff
        const value = this.magic.performFireworks()
        // Do a lot of other stuff with `value`, eventually return something
    }

}

Now I want to write a test for this class, and its behaviour with the magic service. The problem is that in order to test it correctly I have to know the underlying mechanism of the magic service implementation, and what its outputs are going to be:

const thing = new Thing();
// Assuming magic has been magically injected inside
const expectedResultLookingAtTheMagicService = 'Boom!'; // I had to look at the magic service to know this
expect(thing.doStuff()).toEqual(expectedResultLookingAtTheMagicService);

This is bad because I feel this is violating some principles, if the Magic service is going to change how it works and/or its outputs, this test, which is about Thing and not Magic, could break.

However, I could solve it easily if I just provide a mock for the Magic service:

class MockMagic implements IMagic {
    performFireworks(){
        return 'Ok'
    }
}
const thing = new Thing();
// Assuming MockMagic has been injected this time
expect(thing.doStuff()).toEqual('Ok') // Value that I know because is in this test and eventually I know this will be the output from the function

Now this feels more correct to me because no matter what changes the implementation of Magic go though, as long its interface do not change this test will not impacted.

--> Here's my problem: with tests like this, how do I make sure that my class is going to do exactly what is supposed to do with the real Magic implementation?

  • Adding tests for the real implementation of Magic could help.

    Let's pretend that we do, and both Thing and the implementation of Magic has 100% coverage, will I still be sure that they work perfectly together?

  • Sounds like a job for an integration/e2e test.

    That may be a solution but would I not ending up in duplicate a bunch of code and creating massive works? I would likely ending up with two tests similar to the one above, for every class that use this or another service injected

I'm not sure what to do.

14
  • 2
    Your intuition is correct: Testing such shallow class with an injected mock provides no value. At best is a waste of time and at worst it gives you a false confidence in the code.
    – JacquesB
    Commented Feb 2 at 15:43
  • 2
    It's worth asking "Does my testing provide value-for-money?" (to the entity who pays my salary). It's important to remember the reason for testing; testing isn't something that we do because we want to write tests, it's something we do because we want to assure our stakeholders that the software will always meet their needs/expectations, even when the code changes. If a test is not verifying something that relates back to a stakeholder's needs in some way, then it is a worthless test. Commented Feb 2 at 17:46
  • 1
    @FilipMilovanović @JacquesB Maybe my example was poor, I modified the Thing class to be a more complicate one, where it possess some logic that we are interested in testing. Commented Feb 2 at 18:03
  • 1
    This topic has been asked and answered in several variations before on this site, for example here Are integration tests meant to repeat all unit tests?
    – Doc Brown
    Commented Feb 2 at 19:11
  • 1
    ... or here: Do I need unit test if I already have integration test?
    – Doc Brown
    Commented Feb 2 at 19:12

1 Answer 1

1

Now that I've read your question a bit more thoroughly, I think the problem lies in you being unclear about what the public API of the IMagic type is, and how the return value figures in it. (It is not easy to discuss and think about design using vague examples, because design decisions are not based solely on the structure of the code, the specifics actually matter.)

// I had to look at the magic service to know this
const expectedResultLookingAtTheMagicService = 'Boom!';
expect(thing.doStuff()).toEqual(expectedResultLookingAtTheMagicService);

This is bad because [...] if the Magic service is going to change how it works and/or its outputs, this test, which is about Thing and not Magic, could break.

So, here's the key question that everything hinges on: Is the "Boom!" output a part of the public API of IMagic or not? Is it included in the "contract" that describes the guarantees that IMagic makes to its clients? In other words, are all implementers of IMagic required to return this exact value as a way to indicate a successful result of performFireworks()? (If so, the documentation of IMagic would indicate this.)

If the answer is yes1, then your original test is fine, and your modified test is wrong (in that it doesn't actually test the contract between the client and the injected dependency). You didn't have to look inside the magic service, as the "Boom!" is a value indicated by the documentation. Furthermore, any implementations of IMagic that don't adhere to this guarantee would be incorrect, and thus not guaranteed to work with Thing (or any other client of IMagic) - i.e., such implementations would be in violation of LSP. In a certain sense, if an implementation returns something other than "Boom!" to indicate success, it is altering the interface of the type (the result comes from outside the expected set of values).

If the answer is no, then that means that the particular value returned ("Boom!") has no special significance. It is not itself a part of the "contract" of IMagic, and therefore, the logic within Thing cannot rely on the result being "Boom!". Or rather, the developer can choose to do so, but assuming IMagic implementations are free to evolve independently, the Thing class is at risk of breaking.

In such circumstances, the fact that the test will break if the implementation of IMagic changes is a good thing - if written well, the test is essentially a stand-in for client code (in this case, a stand-in for anything that uses the Thing class), and each test case represents a situation that client code could encounter. You don't want your tests to be fragile (you don't want them break for silly reasons), but you do want them to break when your actual production code is also going to break. You want them to fail and warn you when your system is at risk.

Under this scenario, your modified test (with the mock) also doesn't do you much good, again, because it bares no relation to the "contract" of the IMagic type. However, at this point, it seems that the "contract" is reduced to "it could return any string", so there's really no behavior that Thing could rely on that you could test.

Now, moving away a bit from the contrived example, if the set of classes that could provide the service is diverse, with each having a different API (analogous to having several different implementations where one returns "Boom!", a different one "OK", etc.), you'd need to unify the representation somehow in order to be able to write the consumer class in the first place. If you have full control over the code of the service classes, you can just change them. If you don't, then you come up with a unified interface (including there, the set of inputs and outputs), and write a wrapper around each service that implements that interface, "translating" the interaction to and from your preferred API, which gets you back to the first scenario.

In some cases, depending on the details of the problem, it may perhaps make sense to go down a different route, and make the Thing class more generic by parameterizing the success value - e.g., the Thing class could obtain and store it during construction, or otherwise ask for it through the IMagic interface; you'd then do any internal checks against that (as opposed to a hardcoded value), and you'd make use of the same capability in your tests.

P.S. As for mocks, you'd use them in cases where (a) the service class has any sort of nontrivial contract (so, anything that's even slightly more involved than the "it could return any string" from the example above), and (b) where using the real implementation is inconvenient or expensive (e.g. you'd have to set up a database, and make calls to it).

The tests of the Thing class would in this situation separately configure the mock in each test case as part of the test case setup, where the mock would reflect a single specific aspect of the contract that the Thing class reacts to, or otherwise relies on for its own logic. Because all correct implementations of the dependency must adhere to that same contract, you can have some degree of confidence the code in the Thing class is correctly written, and that it will work as expected with different implementations (though you still have to test the system as a whole).

A separate set of tests would exercise the high-level behavior of IMagic implementations (these tests can actually be written against the interface itself, and then different implementations can reuse the same set of tests, though this is not easy to do correctly, and it also isn't particularly straightforward to do in popular testing frameworks).


1 Examples of such methods are those that return an enum, or those that can return special indicator values, such as -1 or null. It doesn't always have to be a specific value, though - it could be a set of rules; take for example the argument to the Array.sort method, the compareFn - a function that takes two values (a and b), and returns a value indicating their relative ordering. Passing a function to another function is not unlike dependency injection. In this case, the "contract" of the compareFn is that it should return a negative value (any negative value) if a < b, zero if they are equal, and a positive value if a > b (where less then, equal, and greater then have meanings that depend on the type of the elements, which could be completely custom objects). The implementation of the Array.sort function relies on this, and therefore a TDD-style test for Array.sort could mock compareFn to exercise a specific scenario. The tests would be quite a bit more involved than the code below, but the spirit of the approach would be the same:

// TEST CASE: If two values are indicated to be out of order, they should be swapped

// For the mock, I'm not even going to look at the values 
// of a and b - they are irrelevant (I'll use dummy values for these anyway). 
// What I really want to test the behavior described above;
// all I need to do here is have the mock simulate that specific scenario.

// Arrange:
let mockCompareFn = (a, b) => 1;  
let arr = ["large_value", "small_value"];

// Act:
arr.sort(mockCompareFn);

// Assert:
// assert that the resulting array is ["small_value", "large_value"];

The idea is to try and write tests in a way that relies more on the high-level rules, on the specifics of the contract between the two interacting objects (or functions), and not so much on specific values that are incidental to that contract. This makes the tests more robust.

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