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 option for filtering out full covered files from HTML report #268

Merged
merged 1 commit into from May 13, 2022

Conversation

albertored
Copy link
Contributor

@albertored albertored commented Sep 21, 2021

I find it useful to focus on files not fully covered so I added an option for HTML reports that allows hiding all fully covered files.

This is only a proposal and some open points/missing things remain to be done but I would like to know your opinion before actually implement all of them.

I know that I can copy the eex template on my repo and add the needed if clause to obtain this behavior but I think this can be useful to other people.

Open points

  • Should I add a disclaimer on the generated HTML that says that fully covered files are hidden?
  • When this option is true should I hide the same files also from the terminal report?
  • Test are still missing

@albertored albertored force-pushed the html-filter-full-covered branch from 220aa91 to e5b5d52 Compare Sep 21, 2021
@parroty
Copy link
Owner

parroty commented Oct 3, 2021

Thank you for the suggestion. The filtering option might be beneficial if it can be switched (as optional). But I'm not confident enough yet, and may need to check the behavior a little more.

@kerryb
Copy link
Contributor

kerryb commented Dec 16, 2021

This would be a very welcome feature. I have a project with a lot of modules, and it can be hard to spot the ones with missing coverage in either the HTML or terminal output.

@albertored
Copy link
Contributor Author

albertored commented May 6, 2022

Hi @parroty, did you have time to think about this PR?

@parroty
Copy link
Owner

parroty commented May 12, 2022

Thank you for the follow-up, and sorry for not being responsive. I think I previously couldn't make the tests pass. Is it possible for you to re-push the changes to run the actions workflow? (I cannot see a re-run option from my end. It could be the previous run was too old).

@parroty
Copy link
Owner

parroty commented May 12, 2022

Thank you for the PR. One request would be, is it possible to add a brief description for the option (html_filter_full_covered) in the README?
Assuming this doesn't break existing behavior (as an optional behavior), I would merge the change once tests pass.

@albertored albertored force-pushed the html-filter-full-covered branch from e5b5d52 to dc8c4dd Compare May 12, 2022
@albertored
Copy link
Contributor Author

albertored commented May 12, 2022

@parroty done

@coveralls
Copy link

coveralls commented May 12, 2022

Coverage Status

Coverage increased (+3.5%) to 90.43% when pulling 6482d8e on albertored:html-filter-full-covered into a716e6e on parroty:master.

@parroty
Copy link
Owner

parroty commented May 12, 2022

Thanks for the quick update. The test failed on the size check part as the HTML layout has changed. Could you update the corresponding test part too?

     stacktrace:
diff --git a/test/html_test.exs b/test/html_test.exs
index 73c8dda..bb29e3f 100644
--- a/test/html_test.exs
+++ b/test/html_test.exs
@@ -5,7 +5,7 @@ defmodule ExCoveralls.HtmlTest do
   alias ExCoveralls.Html

   @file_name "excoveralls.html"
-  @file_size 20191
+  @file_size 20375
   @test_output_dir "cover_test/"
   @test_template_path "lib/templates/html/htmlcov/"

@albertored albertored force-pushed the html-filter-full-covered branch from dc8c4dd to 6482d8e Compare May 12, 2022
@albertored
Copy link
Contributor Author

albertored commented May 12, 2022

Oh sorry, I didn't see that error, now it should be ok

@parroty
Copy link
Owner

parroty commented May 13, 2022

Thanks!

@parroty parroty merged commit e38361a into parroty:master May 13, 2022
4 checks passed
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

4 participants