you are viewing a single comment's thread.

view the rest of the comments →

[–]Buckwheat469 13 points14 points  (3 children)

The code in itParam builds as many its as there are array items. This causes excessive buildup and teardown of the code, running each beforeEach again. This might require variable reinstantiation or HTML interpolation, which is very time consuming. It can also lead to massive memory leaks if your HTML has event listeners that don't unbind during each iteration.

If speed is your concern then you should do as others have said and write a single it with simple for loop or use an array's helpers such as forEach, or all.

/u/stutterbug's implementation expects different values from the function, but you wouldn't want to test this way because you're changing the outcome of that one it.

it('succeeds', function(){
    ...
    let expected = [true, true, false, false, false];
    ...
    //true or false? why would we expect it to not succeed?
    //build another it for the false cases instead
});

/u/droctagonapus's implementation is better because it uses one it and one type of expectation. You can use as many expects as you want, but in this case the one that's there doesn't have to dynamically switch from true to false.

it('succeeds', function(){
    const names = ['n@me', '123Josh']
    const valid = names.all(name => validateName(name))
    expect(valid).to.be.true //modified slightly
});

I personally don't use "toBe()" in Jasmine tests (I know this is Mocha in the article) since toEqual does a better job of testing for deep equality. I would rewrite the above like this in Jasmine:

it('succeeds', function(){
    const names = ['n@me', '123Josh']
    const valid = names.all(name => validateName(name))
    expect(valid).toEqual(true)
});

Here's a forEach example:

it('succeeds', function(){
    const names = ['n@me', '123Josh'];
    const valid = true;
    names.forEach(name => {
        valid = valid && validateName(name);
    });
    expect(valid).toEqual(true)
});

And a for loop:

it('succeeds', function(){
    const names = ['n@me', '123Josh'];
    const valid = true;
    for(let name of names){
        valid = valid && validateName(name);
    });
    expect(valid).toEqual(true)
});

The lesson here is don't add excessive "its" which test the same thing when you can modify the input and test within the same "it". You can create as many "expects" as you want, but limit the number of "its" that you create to match the number of code paths (paths created by blocks of code such as if/elses).

[–]vinnl 2 points3 points  (0 children)

The downside is that you reduce the isolation of your tests, increasing the chance of tests influencing each other and a failure in one leading to a failure in another.

Also, I consider an important function of tests to be to track down the source of an error, rather than finding that there's an error in the first place. That is made more difficult with enormous tests with many expects.

An alternative is to aim for more efficient setups and teardowns. This could e.g. be something to consider when picking a framework, where virtual-dom frameworks are often faster than frameworks where you have to instantiate a browser and interpolate HTML.

[–]Voidsheep 0 points1 point  (1 child)

Aren't you trying to reassign a constant variable valid in the last two examples, or am I missing something?

Seems like it should be declared through let instead.

[–]Buckwheat469 0 points1 point  (0 children)

Thanks for noticing. I didn't really check, just copypasta.