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

Add failing test for conditional value #688

Merged
merged 1 commit into from Mar 14, 2017

Conversation

@Marsup
Copy link
Member

Marsup commented Mar 14, 2017

We have a similar case on our project where a value is assigned based on boolean short-circuits, it shows it as uncovered so we have to convert it to A ? A : B instead of A || B. I looked at it quickly but I'm not sure how to fix it or if we want to fix it at all, as it could detect some actual failures in coverage. I'm PRing the failing test in case someone has an idea.

@geek geek added the test label Mar 14, 2017
@geek geek self-assigned this Mar 14, 2017
@geek geek merged commit 52a7364 into master Mar 14, 2017
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
Node Security No known vulnerabilities found
Details
@geek geek deleted the cond-value branch Mar 14, 2017
@geek geek added this to the 13.0.2 milestone Mar 14, 2017
@Marsup

This comment has been minimized.

Copy link
Member Author

Marsup commented Mar 14, 2017

Hmmm, did you really intend to merge that ? :)

@geek

This comment has been minimized.

Copy link
Member

geek commented Mar 14, 2017

@Marsup ya, extra motivation to resolve the issue... if it doesn't work out I'll revert.

@geek

This comment has been minimized.

Copy link
Member

geek commented Mar 14, 2017

@Marsup

This comment has been minimized.

Copy link
Member Author

Marsup commented Mar 14, 2017

I don't get it, you fixed the failing test which demonstrated the issue but not the code.

@geek

This comment has been minimized.

Copy link
Member

geek commented Mar 14, 2017

@Marsup the test demonstrates that A || B is covered. Is the issue only when B is a default value that never changes?

@Marsup

This comment has been minimized.

Copy link
Member Author

Marsup commented Mar 15, 2017

Not sure, but that's the only difference I see. It might be when it's in another scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.