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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't print coverage violations until tests finish #891

Closed
wants to merge 1 commit into from

Conversation

joshuapinter
Copy link
Contributor

@joshuapinter joshuapinter commented May 3, 2020

If running tests in parallel (e.g. using parallel_tests), coverage and coverage drop violations were being reported prior to the entire test suite finishing.

Two issues with this:

  1. You get violations being shown prior to the entire test suite finishing that are not representative of the entire test suite coverage.

  2. It makes the test results ugly and distracting.

For example, if your minimum line and branch coverage is 50 and 40, respectively, and your final line and branch coverage is 59 and 47, respectively, you will get violations prior to the full test suite finishing.

This commit ensures that if you are running tests in parallel, it only reports the coverage and coverage drop violations on the final process running, after all the tests have run.

Before

Using recorded test runtime
8 processes for 173 specs, ~ 21 specs per process
Line coverage (37.08%) is below the expected minimum coverage (50.00%).
Branch coverage (25.76%) is below the expected minimum coverage (40.00%).
.............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................Line coverage (40.68%) is below the expected minimum coverage (50.00%).
Branch coverage (32.02%) is below the expected minimum coverage (40.00%).
.......................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵馃晵....................................馃晵馃晵......................................................................................................................................................Line coverage (44.46%) is below the expected minimum coverage (50.00%).
Branch coverage (33.26%) is below the expected minimum coverage (40.00%).
................................................................................................................................................................................................................................................................................................Line coverage (48.28%) is below the expected minimum coverage (50.00%).
Branch coverage (34.44%) is below the expected minimum coverage (40.00%).
....................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵Branch coverage (37.17%) is below the expected minimum coverage (40.00%).
...............................................................................................................................................................................................................................................................馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵...............................................................................................................................................................馃晵.......................................................................................................................................Coverage has dropped by 4.64% since the last time (maximum allowed: 2.00%).
............................................................................................................................................................................................................................................................................................................................................馃晵...........................................................馃晵...馃晵馃晵馃晵馃晵..馃晵.................................................................馃晵........................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵...........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵....馃晵馃晵馃晵馃晵馃晵................................................................................................................................馃晵....................................................................................................................................................................................................................................................................

Coverage report generated for (1/8), (2/8), (3/8), (4/8), (5/8), (6/8), (7/8), (8/8) to /Users/user/cntral/coverage. 6662 / 11228 LOC (59.33%) covered.

After

Using recorded test runtime
8 processes for 173 specs, ~ 21 specs per process
..................................................................................................................................................................................馃晵馃晵馃晵馃晵.............................................................................................................................................................................................................馃晵...........................................................馃晵...馃晵馃晵馃晵馃晵..............................................................................................................................................................................................................................................................................................................................................................................................................................馃晵馃晵....................................馃晵馃晵....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵..............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵.....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵.........................................................................................................................................

Coverage report generated for (1/8), (2/8), (3/8), (4/8), (5/8), (6/8), (7/8), (8/8) to /Users/user/cntral/coverage. 6661 / 11239 LOC (59.27%) covered.

If running tests in parallel (e.g. using `parallel_tests`), coverage and coverage drop violations were being reported prior to the entire test suite finishing.

Two issues with this:

1. You get violations being shown prior to the entire test suite finishing that are not representative of the entire test suite coverage.

2. It makes the test results ugly and distracting.

For example, if your minimum line and branch coverage is 50 and 40, respectively, and your final line and branch coverage is 59 and 47, respectively, you will get violations prior to the full test suite finishing.

This commit ensures that if you are running tests in parallel, it only reports the coverage and coverage drop violations on the final process running, after all the tests have run.

## Before

```bash
Using recorded test runtime
8 processes for 173 specs, ~ 21 specs per process
Line coverage (37.08%) is below the expected minimum coverage (50.00%).
Branch coverage (25.76%) is below the expected minimum coverage (40.00%).
.............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................Line coverage (40.68%) is below the expected minimum coverage (50.00%).
Branch coverage (32.02%) is below the expected minimum coverage (40.00%).
.......................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵馃晵....................................馃晵馃晵......................................................................................................................................................Line coverage (44.46%) is below the expected minimum coverage (50.00%).
Branch coverage (33.26%) is below the expected minimum coverage (40.00%).
................................................................................................................................................................................................................................................................................................Line coverage (48.28%) is below the expected minimum coverage (50.00%).
Branch coverage (34.44%) is below the expected minimum coverage (40.00%).
....................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵Branch coverage (37.17%) is below the expected minimum coverage (40.00%).
...............................................................................................................................................................................................................................................................馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵...............................................................................................................................................................馃晵.......................................................................................................................................Coverage has dropped by 4.64% since the last time (maximum allowed: 2.00%).
............................................................................................................................................................................................................................................................................................................................................馃晵...........................................................馃晵...馃晵馃晵馃晵馃晵..馃晵.................................................................馃晵........................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵...........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵....馃晵馃晵馃晵馃晵馃晵................................................................................................................................馃晵....................................................................................................................................................................................................................................................................

Coverage report generated for (1/8), (2/8), (3/8), (4/8), (5/8), (6/8), (7/8), (8/8) to /Users/joshuapinter/Development/cntral/coverage. 6662 / 11228 LOC (59.33%) covered.
```

## After

```bash
Using recorded test runtime
8 processes for 173 specs, ~ 21 specs per process
..................................................................................................................................................................................馃晵馃晵馃晵馃晵.............................................................................................................................................................................................................馃晵...........................................................馃晵...馃晵馃晵馃晵馃晵..............................................................................................................................................................................................................................................................................................................................................................................................................................馃晵馃晵....................................馃晵馃晵....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵..............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵.....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵.........................................................................................................................................

Coverage report generated for (1/8), (2/8), (3/8), (4/8), (5/8), (6/8), (7/8), (8/8) to /Users/user/Development/cntral/coverage. 6661 / 11239 LOC (59.27%) covered.
```
@joshuapinter joshuapinter marked this pull request as draft May 3, 2020 19:10
Copy link
Collaborator

@PragTob PragTob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there!

Thanks for your contribution! 馃帀 馃挌 Sorry for the long silence, I didn't spend much time on OSS in recent times.

It looks like a good solution, the result exit status processing needs a refactor/extraction which I wouldn't want to pin on you. But you'd need to disable the cyclomatic code complexity cop here :)

The PR is fixing it in slightly inconsistent ways though. Namely for Minimum violation only the reporting is disabled but the exit code is still returned. For coverage drop the whole status isn't reported.

Thinking about all of this, we might make it all easier - as long as we're not the final_result process just return success. That'd also catch all future cases and introduce less branches. Could be handled in a guard statement, what do you think?

Thanks a bunch! 馃挌

WhatsApp Image 2019-06-26 at 08 42 20

@PragTob PragTob changed the base branch from master to main July 5, 2020 09:46
@PragTob
Copy link
Collaborator

PragTob commented Aug 11, 2020

Hi there,

I wanted to get back here looking to fix it up but I realized I accidentally fixed this in #906, by only triggering any of this when we are the final process: https://github.com/colszowka/simplecov/pull/906/files#diff-107ed4983c51dd50042b98843dbaad97R194

Thanks for report & input - closing in favor of #906 !

@PragTob PragTob closed this Aug 11, 2020
@joshuapinter
Copy link
Contributor Author

Thanks for following up. I'll pull in your branch and test it out to see what the output looks like. If it fixes the issue, perfecto!

@PragTob
Copy link
Collaborator

PragTob commented Aug 11, 2020

Thanks for checking, it should... it should basically be the same fix but just applied on a wider scope.

I'm writing a test right now to make sure but there I ran into issues with parallel tests... :'(

@joshuapinter
Copy link
Contributor Author

Yup, perfecto!

Using recorded test runtime
12 processes for 178 specs, ~ 14 specs per process
...................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵....馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵馃晵..............................馃晵馃晵....................................馃晵馃晵...........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵馃晵馃晵馃晵馃晵.................................................................................................................................................................馃晵........................................................................................................................................馃晵......................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................馃晵...........................................................馃晵...馃晵馃晵馃晵......................................馃晵.................................................................馃晵......................................................................................................................................................................................................................................................................................................................馃晵.............................................................................................................................................................................馃晵.....................................馃晵..........................................................................................................................................................................................................................................................................................

Coverage report generated for (1/12), (10/12), (11/12), (12/12), (2/12), (3/12), (4/12), (5/12), (6/12), (7/12), (8/12), (9/12) to /Users/user/cntral/coverage. 6899 / 11119 LOC (62.05%) covered.

Any idea when we can expect the exit-status-refactoring branch to be merged into master?

@PragTob
Copy link
Collaborator

PragTob commented Aug 11, 2020

@joshuapinter unless someone does a review destroying my refactor I'll merge it today or tomorrow. It's also my personal goal to cut a release this week 馃

@joshuapinter
Copy link
Contributor Author

Beauty! Thanks for maintaining this. I'll look for the merge commit and then switch my Gemfile over. :)

PragTob added a commit that referenced this pull request Aug 11, 2020
When I accidentally fixed something, I hope I didn't accidentally
break things.
PragTob added a commit that referenced this pull request Aug 12, 2020
When I accidentally fixed something, I hope I didn't accidentally
break things.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants