Skip to content
This repository was archived by the owner on Dec 8, 2024. It is now read-only.

branch coverage of && not perfect #189

Open
Raynos opened this issue Apr 10, 2014 · 6 comments
Open

branch coverage of && not perfect #189

Raynos opened this issue Apr 10, 2014 · 6 comments

Comments

@Raynos
Copy link

Raynos commented Apr 10, 2014

I have a branch that is a && !b

It makes that branch as 100% coveraged even though b is always false.

It never says a && !b is not covered because b is never true.

I suspect that we should really have 4 cases here, for (true, true), (false, false), (true, false) and (false, true).

If any of thoses cases are covered it seems unfair to say its 100% branch covered.

@Raynos
Copy link
Author

Raynos commented Apr 11, 2014

Another example

opts = opts || {};
opts.maxBuffer = opts.maxBuffer || 2000 * 1024;

In this piece of code opts is never falsey so it correctly marks || {} as uncovered.

However opts.maxBuffer is ALWAYS falsey so it uncorrectly marks opts.maxBuffer as covered (it also correctly marks || 2000 * 1024 as covered).

@gotwarlost
Copy link
Owner

Yes, these are known issues - I believe there is another issue for the or case. Need to think about this.

@Swatinem
Copy link

However opts.maxBuffer is ALWAYS falsey so it uncorrectly marks opts.maxBuffer as covered (it also correctly marks || 2000 * 1024 as covered).

Even if its falsy, it is still covered (executed), otherwise you won’t get to the || part.
Essentially the first part of a && or || is always covered.

Or am I getting something wrong here?

@piuccio
Copy link
Contributor

piuccio commented May 26, 2014

The first part is covered in terms of statements coverage, not if you're speaking of branch coverage.

Well it's covered only partially, the truth-y branch in this example

@darrell-newman
Copy link

Considering a && b...

If a is falsey then b is never evaluated, it may even be an error to do so. So the (falsey, *) cases both reduce down to one.

Further, the && operator does not care about the truthiness of its second argument, so it isn't appropriate to judge it in terms of whether b is truthy/falsey.

However...

&& is most often used in conditionals, in which the value of b is reduced to its truthiness. What's needed is better conditional coverage rather than branch coverage.

So for this:

a && b

I would say there are only 2 cases: a is falsey or a is truthy.

But for this:

if (a && b) {

I would say there are 3 cases: (falsey, *), (truthy, falsey) and (truthy, truthy). Currently the first two can't be distinguished.

@nice-shot
Copy link

Just wanted to know if there was any update regarding this.
Specifically the or operator (||) branch seems necessary to recognize since it is often used in option parsing. For example:

function Foo (options) {
  options = options || {};
  this.options = {
    bar: options.bar || 0,
    baz: options.baz || 1
  };
}

If I just create one instance of Foo in my test without ever adding options it will be considered 100% covered.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants