1

I've come in to a project where it's a best practice for an Init method for all tests in a class to contain mocked happy path functionality. This should allow most methods to pass without exceptions. I'm guessing the benefit is removal of code duplication and that other test methods not directly related to this code execution won't be changing with it.

I'm not used to something like this and I have some reservations:

  1. Test is easier to read if we can follow the flow of code inside of it
  2. We can potentially get some unexpected behavior ?

Normally I would just add the mock return value for each tested method where its necessary. Example below using some pseudo code

[TestFixture]
public class CLassToBeTestedTest
{
    private Mock<IExampleInterface> _someMethodToBeMocked;
    private CLassToBeTested _underTest;

    [SetUp] //Init that will be called for all the other tests as well before they run
    public void Setup()
    {
        _someMethodToBeMocked = new Mock<IExampleInterface>();
        _underTest = new CLassToBeTested(_someMethodToBeMocked.Object);
    }


    [Test]
    public async Task TestedMethodName_HappyPathCondition_HappyPathResult()
    {
        // Arrange | ## Mocked method setup is part of Arrange of a tested method ##
        _someMethodToBeMocked.Setup(a => a.SomeValidation().ReturnsAsync(true);

        // Act
        var response = await _underTest.MethodToBeTested();

        // Assert
        Assert.AreEqual(expectedValue: true, response);
    }

    //... some other tests
}

public class CLassToBeTested
{
    private readonly IExampleInterface _exampleInterface;

    CLassToBeTested(IExampleInterface exampleInterface)
    {
        _exampleInterface = exampleInterface;
    }

    public bool MethodToBeTested()
    {
        if (!_exampleInterface.SomeValidation())
            return false;

        return true;

    }
}

Now the proposed best practice is that for all the mocked methods their happy path will be put inside the Setup() method which will be run before all the tests

[TestFixture]
public class CLassToBeTestedTest
{
    private Mock<IExampleInterface> _someMethodToBeMocked;
    private CLassToBeTested _underTest;

    [SetUp] //Init that will be called for all the other tests as well before they run
    public void Setup()
    {
        // Mocked method returns a result for all the tests. If not happy path it needs
        // to be overwritten in the test class
        _someMethodToBeMocked = new Mock<IExampleInterface>()
            .Setup(a => a.SomeValidation().ReturnsAsync(true);

        _underTest = new CLassToBeTested(_someMethodToBeMocked.Object);
    }


    [Test]
    public async Task TestedMethodName_HappyPathCondition_HappyPathResult()
    {
        // Arrange | ## Arrange is empty. Its code is moved to Setup() if considered happy path ##


        // Act
        var response = await _underTest.MethodToBeTested();

        // Assert
        Assert.AreEqual(expectedValue: true, response);
    }

    //... some other tests
}

public class CLassToBeTested
{
    private readonly IExampleInterface _exampleInterface;

    CLassToBeTested(IExampleInterface exampleInterface)
    {
        _exampleInterface = exampleInterface;
    }

    public bool MethodToBeTested()
    {
        if (!_exampleInterface.SomeValidation())
            return false;

        return true;

    }
}

I was wondering if you have some input regarding this topic whether it should be considered a best practice.

1
  • 3
    A best practice for what? Everything is a trade-off, there are no absolutes. Commented Mar 15, 2022 at 13:47

4 Answers 4

12

"Best practice" always sounds to me like "braindead practice", because it gives the impression of a universal rule one can apply without further thinking. I think a better way of asking would be when does the approach presented in the question make sense, and when not, so let me try to answer this.

It is not a bad idea in general to have a central piece of code providing some "default behaviour" for the most frequent case, which makes it only necessary to write extra code for less frequent cases with some other behaviour. That is a common coding strategy for avoiding boiler plate code, regardless whether the code is about tests or other things.

What you should check here is if the "happy path" is really the "80% case", and that it really requires the same validation setup in several tests. For example, maybe there are a lot of tests for this "happy path", but in case you can implement them in a data-driven manner using just one test function, then putting the default validation setup into the central "Setup" function would be less justified.

The drawbacks of the approach are that the "default Arrange" code will be located less near to the "Act" and "Assert" parts, and that it will become a little bit more effort in case one needs to change the "default behaviour" afterwards, since this will affect more than one test. So if you think these (small) tradeoffs are worth the benefits, then go ahead.

I would also think about an alternative: putting the line

  _someMethodToBeMocked.Setup(a => a.SomeValidation().ReturnsAsync(true);

into a reusable method (i.e. DefaultArrangeForTrueValidation()), and call this method at the beginning of each test where it applies. This is also a DRY solution (though it may require more boilerplate), it is more explicit than your proposed solution of putting this code directly into Setup(), and more suited if you have both: several tests with this "default arrangement", and several other tests with a different arrangement.

IMHO the best way of finding out which approach will suit you best is by implementing a few different tests and refactor whenever you find duplicate logic at least two, three or four times times.

In short:

Beyond your own approach
  • moving duplicate code to the [SetUp] method

for making the code more DRY, there are alternative approaches like

  • refactoring common logic into a separate function
  • data driven tests

Put them all into your toolbox and pick the one which suits you best in the specific situation. And do yourself a favor and avoid thinking in "best practices", this makes every problem look like a nail just because you found a hammer.

1
  • 1
    This answer is exactly what I was thinking, too. Something else to consider: split the test class into multiple classes when the common setup and setup for each test method becomes a mess. Messy, disoriented and difficult-to-read setup can indicate the test class itself is testing too much behavior (violating SRP, perhaps?). Smaller test classes where the common setup can be expressed in one place becomes desirable in cases like that -- but only if each test in the class is cohesive. Common setup code doesn't necessarily mean the tests belong in the same class. Commented Mar 16, 2022 at 12:45
2

I have seen this and similar mocking practices recommended, but I agree with your objections. It's not a great idea to have 'hidden' mock setup outside of the test for the following reasons:

  1. What about 'non happy' tests, eg MethodReturnsFalseWhenValidationFails()? Now you have to override the default setup somehow.

  2. What about tests which unknowingly use the default setup to pass? eg OtherMethodReturnsTrueWithNoValidationRequired() this now passes with a false positive

  3. You end up with more test setup code than actual code. As you try to write these setup methods in reusable and nicely named ways you end up writing (and debugging) your own test framework code, rather than tests.

It's generally better to stick with Arrange Act Assert in each test, even if this means repeated code. There are plenty of off the shelf testing and mocking libraries you can use to keep your Arrange short and readable

1

Any code in Setup will be run before every test. Happy or not. Therefor you can only put stuff in there that can be run before every test. Any code in there that is pointless to run for some tests is distracting. But it's also distracting if you have to paste it in nearly every test.

How much tolerance you should have for repeated code depends on the situation. But always strive to get to the point quickly. Seeing the same code over and over every time slows me down. But wondering where the magic is coming from also slows me down. So keep the repeated code small and keep the magic easy to explain.

As for making the "happy path" a priority when selecting what to put in setup, well that sounds fine. Making it a fanatical obsession to be followed while ignoring all other concerns, well that's not fine.

1

The answer to your direct question

If the act itself is sufficiently complex, and there is value to its reusability, I see no reason to condemn this approach.

As is often the case, the question isn't so much whether there is a reason to not do this, but rather if there is no reason to do it. These two are subtly but importantly different.
This means that your question is hard to answer; because I cannot give you conclusive evidence that it shouldn't be done. The onus is on you here to provide sufficient reason for doing it. In the absence of such reason in your question, I can only guess as to whether there is none or whether you forgot to mention it.

  1. Test is easier to read if we can follow the flow of code inside of it

I mentioned that the act in question need to be sufficiently complex. A trivial one-liner ("trivial" is a key concept here - not just any one-liner) shouldn't need abstraction, but a more complicated snippet can benefit from it.

In this sense, the complexity of your code is assumed to be a detriment to the readability of the method, hence why abstracting it away would benefit your readability.

Your argument seems to be based on "if I see it, I understand it". While this is correct for trivial things, it is not correct for complex things. For complex things, abstraction helps hide away all the implementation details that don't matter on a higher level.

  1. We can potentially get some unexpected behavior ?

Generally speaking, unexpected behavior stems from a developer's inability to keep track of all the proverbial balls they're juggling. They drop one, which leads to unexpected behavior. Had they had their eye on that ball, they would not have developed that behavior to be the way that it is.

Tests, however, have a high inherent standard for understandability. Almost inherently, a test requires you to keep your eye on every single ball, because otherwise you can't even know if this test is meaningful, let alone correct.

This refers back to the previous point. The higher the readability, the less chance of a developer not keeping an eye on one of the proverbial balls, which in turn lessens the chance of unexpected behavior. When the abstraction increases the overall readability, it will contribute positively towards the goal of avoiding unexpected behavior.


The answer I think you need

Before the next point I want to make, I want to point out that I cannot conclusively judge your ability as a developer, nor am I trying to do so. However, I do want to point out that the basis of your argument is one that I have encountered before with other developers.

Based on your concerns and argumentation about the proposed testing format, it seems to me that you clearly like having everything in view so that you can keep track of it all, and you worry about abstraction as it obscures implementation details from your view.

This is not the first time I've encountered this kind of reasoning. In my opinion, this is a roadblock many (if not all) of us have to cross somewhere on our road from junior to senior developer (some people encounter it sooner than others), but this does require a mindset shift to solve.
It will require effort to learn and get used to a new way of doing things, but the sooner you identify the issue, the sooner this shift can happen and benefit your daily experiences. The reason I'm bringing this up is to hopefully help with identifying the issue and how to best tackle it, so that you can get on that road (again: I cannot conclusively judge you as a developer, but my gut tells me you're at this particular roadblock).

The issue here is one of taking on too much all at the same time. In the beginning, as a junior developer, you work with relatively trivial and straightforward code. Adding additional abstractions to this code costs more effort than the benefit you get from them is worth.

Going back to my analogy, simple code snippets are easy balls to juggle, and therefore you can juggle many of them at the same time. However, as your experience as a developer increases, you tend to deal with more complex implementations. These balls get gradually harder to juggle.

If this increase is gradual and subtle, it's very easy to subconsciously try to put an increasing amount of effort into juggling the same number of balls as you did before. This is a well-studied psychological effect where people do not notice subtle changes, e.g. to the weight of something they're carrying, and therefore they never think to adjust their approach.

However, you will reach a breaking point where the implementation becomes so complex that it simply cannot be done - on the senior end of the developer spectrum. We're all different people and we all have different limits; but the limit is there. Even if you have not reached your limit yet; you're still putting in more effort to compensate for the added complexity, and if you stick with this approach the only logical next step is having to put in even more effort as the balls get harder to juggle.

There is a second solution. Instead of putting in more effort to juggle these increasingly difficult balls; we could find a way to reduce the amount of balls we have to juggle. This way, we can dedicate a large portion of our total effort into each individual ball, while at the same time keeping the total amount of effort the same.

This is what abstraction does for you. It separates your juggling balls so that you do not have to juggle all of them at the same time.

Concretely, for your scenario, the separation of the setup and the actual tests makes it so that the setup does not interfere with your cognitive load when you are designing your test. This is one less ball to juggle, and that regained effort can therefore be dedicated towards the design of the test, not the specific implementation of a mocked behavior.

Think of it like someone who is trying to paint an artwork; but at the same time also has to wash his brushes, make new paint brushes or fix worn paint brushes, and make paint from pigments. These secondary tasks distract them from the primary one, and it will negatively impact the result (or require more total effort to get there).
It would be beneficial if the artist relied on existing paint and brushes provided by another party; so that the artist could focus on the artwork itself and nothing else.

As I mentioned earlier, the abstraction only makes sense in cases where the abstraction decreases the complexity; which only applies where the concrete code is sufficiently complex to begin with. Washing a paint brush is probably not worth the abstraction, but if the process of making/fixing paint brushes and mixing paint is sufficiently complicated; it will positively impact the artist to no longer be sidetracked by those tasks.

I'm not used to something like this and I have some reservations

It is good that you critically think about a new thing that you're being introduced to. The fact that you asked this question suggests that you have a healthy approach to learning and acquiring insights.

But in this case, the better approach is to embrace the new system. In the beginning it will be a blind embrace, but over time you will see and understand its benefits. Much like how the juggling balls got gradually harder to juggle, this new approach will gradually decrease the number of balls you're juggling.

You might not notice immediately, but you'll definitely get to a point where you realize that you're juggling considerably less balls than you used to.

Note that a blind embrace is not the same as a dogmatic overapplication of a rule. As mentioned before, there will always be cases where abstracting trivial code is not a helpful approach. The trick is to identify where abstraction positively contributes, and only apply it there.
The implementation of an abstraction is part of a medior developer's skillset. Knowing when to implement an abstraction is part of a senior developer's skillset.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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