0

After a lot reading, I found that people advice to mock every dependencies in a method under unit test.

Should I mock all the dependencies when unit testing?

When should I mock?

However, I have a method that creates a list of objects (the dependencies) and uses the method these objects to alter the list. In the real-life scenario, the method is transforming data payload sent by the client into these objects. Below is the code implementation in Typescript but it just gives you an idea.

import {expect} from 'chai';

import 'mocha';

class Bar {
    private val: number;

    constructor(val: number) {
        this.val = val;
    }

    isValid(): boolean {
        return this.val !== 2;
    }

    canMergeWith(bar: Bar): boolean {
        return this.val === bar.val;
    }
}

class BarBuilder {
    constructor() {
    }

    createBars(payload: number[]): Bar[] {
        const bars: Bar[] = [];
        payload.forEach(p => {
            bars.push(new Bar(p));
        });

        const filtered = this.filterBars(bars);
        const merged = this.mergeBars(filtered);
        return merged;
    }

    private filterBars(bars: Bar[]): Bar[] {
        return bars.filter(b => b.isValid());
    }

    private mergeBars(bars: Bar[]): Bar[] {
        const merged: Bar[] = [];
        bars.forEach((bar, i) => {
            if (i > 0) {
                const before = bars[i - 1];
                if (!bar.canMergeWith(before)) {
                    merged.push(bar);
                }
            } else {
                merged.push(bar);
            }
        });
        return merged;
    }
}

describe('BarBuilder', () => {
    it('Should convert payload into valid bars', () => {
        const payload = [1, 2, 3, 4, 4, 5, 5];
        const barBuilder = new BarBuilder();
        const bars = barBuilder.createBars(payload);
        expect(bars).to.deep.equal([{val: 1}, {val: 3}, {val: 4}, {val: 5}]);
    });
});

Currently, the test is passing. But it is violating the rule that I should mock every dependency in the class.

My question which one of the following should I do:

  1. Should I really mock the class Bar in order to comply to that rule?

If so, I find another difficulty doing that. If I mock the class Bar, how do I make the mock methods canMergeWith and isValid return true or false based on their input? Isn't the equivalent as re-writting the class itself?

  1. Should I refactor my code instead?

I was also thinking probably the problem is with my code being not testable. Before, the methods canMergeWith and isValid belonged to BarBuilder. For the sake of readability, I moved them inside Bar. But now, the unit testing BarBuilder is not trival anymore .

  1. Leave the test as it is despite the fact that it is violating unit testing rule.
TSR
  • 9,145
  • 14
  • 51
  • 114

1 Answers1

2

I found that people advice to mock every dependencies in a method under unit test.

Some people advise this, yes. In the linked threads, more people disagree. Unit testing is actually a divisive subject.

It surprised me the first time I learned there is no definition of unit testing that is widely accepted across the industry. People often talk about unit testing as if there is one definition, especially the "mock everything" definition; but if you read through the linked threads, you will see numerous pitfalls of that approach, especially the fragility of those tests.

I think the "mock everything" definition remains so pervasive due to how trivial it is. There's no thinking involved, just mock everything. The alternative is to consider when to mock and when not to. That's harder. You have to justify mocking. It's easier to define unit testing as "mock everything" because then you don't think about what purpose the mocking serves. It's just part of the definition.

Ian Cooper has given a talk on unit testing, as defined by Kent Beck in the context of TDD. I find his perspective to be more modern than the "mock everything" approach.

My advice: create mocks when they help you. Don't create mocks to satisfy a rule. There is no rule with consensus behind it.

jaco0646
  • 11,033
  • 7
  • 47
  • 64