Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Possible) Bug in Lab Coverage Checker ? #401

Closed
nelsonic opened this issue Jul 9, 2015 · 11 comments
Closed

(Possible) Bug in Lab Coverage Checker ? #401

nelsonic opened this issue Jul 9, 2015 · 11 comments
Assignees
Labels
non issue Issue is not a problem or requires changes

Comments

@nelsonic
Copy link

nelsonic commented Jul 9, 2015

Hi @geek
Hope your day is going well.
I'm in the process of writing a (Fully Tested) Hapi.js + Socket.io + Redis Pub/Sub Example/Tutorial app... see: https://github.com/dwyl/hapi-socketio-redis-chat-example/

But, sadly I'm unable to get Lab code coverage checker to cooperate...

Lab coverage is convinced that a few lines of my code are not being covered by my tests, when they clearly are being executed!!

how-does-lab-think-this-line-is-not-bing-covered
redis_config-js-showing-lines-4-7

I don't understand ... 😢
If you (or anyone else in the illustrious list of contributors) can give me some insight I'd be super grateful!

Git Commit: dwyl/hapi-socketio-redis-chat-example@a8722bb

nelsonic added a commit to dwyl/hapi-socketio-redis-chat-example that referenced this issue Jul 9, 2015
@nelsonic nelsonic changed the title Bug in Lab Coverage Checker (Possible) Bug in Lab Coverage Checker ? Jul 9, 2015
@dbashford
Copy link

Having a similar issue. I switched from supertest/mocha+chai/istanbul and 100% coverage to server.inject/lab/lab and 99% without changing any code other than moving some tests around. Thinking the coverage is having issues with complex booleans?

https://coveralls.io/builds/3021647/source?filename=src%2Froute_builder.js

So getting knocked on the ifs, for instance, but not the content of the block?

@nelsonic
Copy link
Author

@dbashford thanks for your quick reply and for confirming I'm not going insane!
I too had 100% test coverage when using Istanbul, but switched to Lab because its the preferred testing framework for Hapi.js apps (which is what demo app I'm writing uses...)

The if(value_is_set) statement in the screenshot above is clearly being executed and both branches are being followed (hence the console.log is being printed - during the test!) I don't understand why Lab thinks the lines don't have coverage ... I don't consider if(value_is_set) to be "complex" code blocks ... 😿

What did you end up doing to re-gain your 100% test coverage?
I've tried adding more tests, but alas nothing seems to work.

Is this a "Bug" or a "Feature"...?

@AdriVanHoudt
Copy link
Contributor

@nelsonic is there a test where the if block is not executed?

@hueniverse
Copy link
Contributor

Is it possible the file is loaded and executed before lab does its thing?

@geek
Copy link
Member

geek commented Jul 10, 2015

@nelsonic does the html report provide any additional clues, perhaps the conditional isn't needed?

@nelsonic
Copy link
Author

The lines in question are: https://github.com/dwyl/hapi-socketio-redis-chat-example/blob/a8722bbe4b641a187e2c0013312e6fbffd8c4496/lib/redis_config.js#L4-L12

And the test which executes them:
https://github.com/dwyl/hapi-socketio-redis-chat-example/blob/a8722bbe4b641a187e2c0013312e6fbffd8c4496/test/_redis_cloud.js#L14-L16

The test is simply loading a configuration file redis_config.js which will follow the first branch of the if because the process.env.REDISCLOUD_URL is indeed set.

We make sure to uncache the file on line 13 https://github.com/dwyl/hapi-socketio-redis-chat-example/blob/a8722bbe4b641a187e2c0013312e6fbffd8c4496/test/_redis_cloud.js#L13 so we are sure that the correct branch will be executed.

We explicitly console.log on line 5
https://github.com/dwyl/hapi-socketio-redis-chat-example/blob/a8722bbe4b641a187e2c0013312e6fbffd8c4496/lib/redis_config.js#L4-L5
inside the branch of the if which is shown in my original screenshot for this issue.

(thanks for helping investigate this...!)

nelsonic added a commit to dwyl/hapi-socketio-redis-chat-example that referenced this issue Jul 11, 2015
@geek geek added the bug Bug or defect label Aug 25, 2015
@mbargiel
Copy link

I ran into an issue similar to this, perhaps this exact issue. It happens to me with lab 10.3.0 (no other dependencies needed).

The problem, I believe, is when an if conditional is checked for only true or false, when there is no else. In that case, we don't expect to have to also test the false condition, because there is no false path.

So assuming the following code:

// some stuff
if (myCondition) {
    // conditional stuff
}
// more stuff

Then assuming this code is tested with either myCondition or !myCondition, but not both, lab will report that the myCondition is not covered. I find this to be confusing, especially considering myCondition has been evaluated.

See the attached files for a very simple repro case.
if_condition_coverage_issue.zip

To run the test cases:
Full coverage: npm run test100
Missing coverage: npm run test50a (if only checked in true branch) and npm run test50b (if only checked in false branch).
For convenience, on windows: npm run test50a_win and npm run test50b_win.

@Marsup
Copy link
Contributor

Marsup commented Mar 17, 2016

@mbargiel That's the expected behavior, the line is covered but if it's always true or always false what's the point of having it at all ?

@mbargiel
Copy link

While writing this feedback, I figured as much. Maybe the problem is how this information is reported, but I have no solution to offer, since the only output is basically a list of line numbers. I guess lab users "just have to know".

In my case, I may have to disable coverage for that if (because the if is always true in tests, and sometimes false outside of tests).

@geek
Copy link
Member

geek commented May 17, 2016

@mbargiel this is now fixed in the html reporter, which shows parts of a line that can be removed

@geek geek closed this as completed May 17, 2016
@geek geek added non issue Issue is not a problem or requires changes and removed bug Bug or defect labels May 17, 2016
@geek geek self-assigned this May 17, 2016
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
non issue Issue is not a problem or requires changes
Projects
None yet
Development

No branches or pull requests

7 participants