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 to change threshold in HTML #261

Closed
Spacetown opened this Issue Jun 14, 2018 · 9 comments

Comments

Projects
None yet
2 participants
@Spacetown
Contributor

Spacetown commented Jun 14, 2018

It would be helpful if the thresholds for medium_coverage and high_coverage could be changed in the command line.

Thanks.

@latk

This comment has been minimized.

Member

latk commented Jun 14, 2018

That's a great suggestion! If you'd like to, you could try implementing this yourself:

  1. set up a development environment as described in the CONTRIBUTING.rst document
  2. add the options to the argument parser in gcovr/__main__.py
  3. update all references in gcovr/html_generator.py to use the options object
  4. change a test case to use custom coverage thresholds

How do you think should the command line options be named? I'd suggest --html-medium-coverage. Or would it be better to have -low- and -high- options, where coverage under the low threshold is marked red?

@Spacetown

This comment has been minimized.

Contributor

Spacetown commented Jun 14, 2018

@latk

This comment has been minimized.

Member

latk commented Jun 14, 2018

I'll trust you to pick a sensible design, as long as you can explain why it is sensible.

If medium is lower than high this should result in a error.

Umm, I assume you meant this the other way around, so that medium <= high must always hold? If the two thresholds are the same, this would suggest that no middle coverage (yellow) would be used, only red & green. This also follows nicely from the current implementation of the coverage_to_color() function.

Excluding impossible levels from the legend is a good idea, This should be simple as you'll only need to change a few lines in the root_page template (they use Jinja2 syntax). However, the template should continue to produce identical output for the existing tests so that we don't have to update all the HTML reference files.

@Spacetown

This comment has been minimized.

Contributor

Spacetown commented Jun 14, 2018

You are right, medium <= high must always be true.
At the moment I'm setting up the environment but I get a error with gcovr\tests\shared_lib\testApp\test\a.out because of missing libs.dll which is in the directory gcovr\tests\shared_lib\lib\libs.dll. The path is:
PATH/c/User/F29233/GitRepositories/gcovr/gcovr/tests/shared_lib/lib:c:\app\tools\_install\tools\python\3_4_1;c:\app\tools\_install\tools\python\3_4_1\Scripts;v:\tools\pc\mingw\bin;v:\tools\pc\mingw\msys\1.0\bin;....
Are the test not running under Windows?

@latk

This comment has been minimized.

Member

latk commented Jun 14, 2018

The tests are a bit flaky on Windows but should work, and we run (nearly) the complete test suite on Windows using AppVeyor. The exact test environment is described in the appveyor.yml file, incl. various environment variables. Maybe you can find a hint there?

For debugging, it might be helpful to see the full output of the build process, e.g. from running this command in Bash:

LANG=C python3 -m pytest --verbose --capture no -k 'shared_lib and txt'

Unfortunately, I'm not deeply familiar with Windows and MinGW. As a random guess, the PATH variable might be wrong by mixing : and ; as entry separators. Do the problems disappear if you fix the Makefile to use ; in the Windows branches?

 txt:
 ifeq ($(BASE_OS),MSYS_NT)
-	PATH="`pwd`/lib:${PATH}" testApp/test/a.out
+	PATH="`pwd`/lib;${PATH}" testApp/test/a.out
 else
 	LD_LIBRARY_PATH=`pwd`/lib testApp/test/a.out
 endif
 	$(GCOVR) -d -o coverage.txt

If the tests don't work locally, that's not a huge problem: you can fork this repository on GitHub, then sign up for Travis CI or AppVeyor so that they run the tests whenever you push to your fork. The AppVeyor tests tend to give results faster.

@Spacetown

This comment has been minimized.

Contributor

Spacetown commented Jun 15, 2018

With another version of MSYS and GCC the tests are running except cmake_oos which selects the Visual Studio compiler on my computer.
The change is done, I've added following options:

"--html-title",
help="Use TITLE as title for the HTML report."
default="Head"
"--html-medium-threshold",
type=check_percentage,
help="If the coverage is below MEDIUM, the value is marked "
     "as low coverage in the HTML report. "
     "The value has to be lower than or equal to --html-high-threshold. "
     "If the value is equal to --html-high-threshold the the report has "
     "only high and low coverage."
default=75.0
"--html-high-threshold",
type=check_percentage,
help="If the coverage is below HIGH, the value is marked "
     "as medium coverage in the HTML report. "
     "The value has to be greater than or equal to --html-medium-threshold. "
     "If the value is equal to --html-medium-threshold the the report has "
     "only high and low coverage.",
default=90.0

and also a testcases for each option. Do I need a fork for a pull request?

@latk

This comment has been minimized.

Member

latk commented Jun 15, 2018

Thank you for your work! Yes, a pull request through GitHub would be easiest. You can click on the fork button, then add your GitHub fork as a remote repository in your local git repository. The repository URL is visible in the "clone or download" button on GitHub, and differs depending on whether you want to use HTTPS+passwords or SSH keys for authentication. For example:

# rename the main gcovr repository from "origin" to "upstream"
git remote rename origin upstream
# add your fork as "origin"
git remote add origin https://github.com/Spacetown/gcovr.git

After you git push your changes to your GitHub fork, you can follow the UI to create a pull request. This will run the full test suite, and allows me to add review comments to your changes.

Regarding cmake, our AppVeyor tests define the environment variable CMAKE='cmake -G "MSYS Makefiles" ' to avoid visual studio. If that fixes the tests for you, I'll amend the contribution docs.

@Spacetown

This comment has been minimized.

Contributor

Spacetown commented Jun 15, 2018

The cmake_oos test did not run on my machine even when changing the cmake call directly in the test.
BTW when is the next release planned?

@Spacetown

This comment has been minimized.

Contributor

Spacetown commented Jun 15, 2018

New pull request #264

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