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

bring test coverage to 100% #1258

Closed
rien opened this issue Sep 7, 2019 · 5 comments
Closed

bring test coverage to 100% #1258

rien opened this issue Sep 7, 2019 · 5 comments
Labels
chore Repository/build/dependency maintenance wontfix This will not be worked on

Comments

@rien
Copy link
Member

rien commented Sep 7, 2019

I like the idea of this blogpost.
TL;DR: a project should have a test coverage of 100%. Not all lines should be tested, but lines we don't want to write a test for should be explicitly marked as such. We can ignore lines like this:

# :nocov:
def skip_this_method
  never_reached
end
# :nocov:

I went trough our coverage report and we are currently just below 80%. There are some lines that we really want to cover (institutions_controller.rb is currently completely uncovered) but some are simply impossible to cover (lib/SAML/my_resource_validator.rb for example).

I suggest someone goes trough this report (found in coverage/index.htm after running rails test) and for each block of uncovered code:

  • either writes a test covering that block
  • either surrounds that block with # :nocov: <Reason why this block is skipped>

Finally, by using SimpleCov.minimum_coverage 100 we can enforce that this being done for each line of extra code that is added to our project.

@bmesuere, @charvp what are your opinions? I think this would be an ideal issue for a job student.

@chvp
Copy link
Member

chvp commented Sep 7, 2019

Coverage reports are broken right now until simplecov-ruby/simplecov#718 is fixed.

@rien
Copy link
Member Author

rien commented Sep 7, 2019

Ah, I have a temporary fix on feature/description-iframe: 2cebcbc which can be used.

Should I make a separate PR with this fix?

@chvp
Copy link
Member

chvp commented Sep 7, 2019

You can already set PARALLEL_WORKERS=1 if you really want serial test execution.

@pdawyndt pdawyndt changed the title Bring test coverage to 100% bring test coverage to 100% Sep 7, 2019
@bmesuere
Copy link
Member

bmesuere commented Sep 7, 2019

+1 for this idea, but we must make sure that they are meaningful tests and not something to reach 100% coverage and be done with it

@rien
Copy link
Member Author

rien commented Sep 7, 2019

You can already set PARALLEL_WORKERS=1 if you really want serial test execution.

Oh, nice. I'll remove that commit then.

@chvp chvp added this to the untriaged milestone Sep 12, 2019
@chvp chvp added the chore Repository/build/dependency maintenance label Jan 26, 2020
@chvp chvp closed this as completed Jun 5, 2020
@bmesuere bmesuere added the wontfix This will not be worked on label Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Repository/build/dependency maintenance wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants