Skip to content

Conversation

@chemelnucfin
Copy link
Contributor

Not sure if this is "desired", but it does make the coverage to be 100%.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 7, 2017
@dhermes
Copy link
Contributor

dhermes commented Dec 7, 2017

@chemelnucfin This is not needed. nox -s cover combines coverage from the nox -s unit runs for ALL versions, so it will hit both branches.

@chemelnucfin
Copy link
Contributor Author

But when you test just a single version, you will wonder what you are missing.

@dhermes
Copy link
Contributor

dhermes commented Dec 7, 2017

That's not the point?

@chemelnucfin
Copy link
Contributor Author

But it does require extra time to confirm that it wasn't what you did that lowered the coverage, right?

@dhermes
Copy link
Contributor

dhermes commented Dec 7, 2017

@chemelnucfin I don't follow. Run nox -s unit cover?

@chemelnucfin
Copy link
Contributor Author

Is there something wrong with the test though? It does fix the coverage issue.

@dhermes
Copy link
Contributor

dhermes commented Dec 7, 2017

Is there something wrong with the test though?

It's the tail wagging the dog. We (our code) shouldn't work around our tools / language versions, they should work around us.

It does fix the coverage issue.

There is no coverage issue to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants