Skip to content

Conversation

mwetter
Copy link
Member

@mwetter mwetter commented Dec 20, 2019

This closes #253.

Todo: @MichaMans, can you please address the following:

  • Replace print with a call to self._reporter.writeOutput. See for example regressiontest.py, line 2634. This way, the message will also be in the log file.
  • I would prefer to call the new method printCoverage rather than get_models_coverage because it only prints something, it does not get anything. Also, we typically use camel case for function names (although not consistently).
  • Add doctest (and ideally a unit test).

A cleaner implementation would probably be to have a method _getCoverage that returns the coverage and everything you need to print, and a wrapper method printCoverage that calls _getCoverage and prints the coverage. Then, you can also write a unit test for _getCoverage to make sure it won't break when people work on BuildingsPy, and you can put the coverage in other (web) documentation that you want to serve from your CI environment. At the minimum, you should add some doctest in its documentation so it is clear how to use it and it will be tested with make doctest

FWuellhorst added a commit to FWuellhorst/BuildingsPy that referenced this pull request Jun 17, 2024
mwetter pushed a commit that referenced this pull request Jun 21, 2024
* add coverage script from Mans with changes based on review in #315 #243

* add unit-test and fix minor bug

* Catch case for no examples
@mwetter mwetter mentioned this pull request Jun 21, 2024
mwetter pushed a commit that referenced this pull request Jun 28, 2024
* add coverage script from Mans with changes based on review in #315 #243

* add unit-test and fix minor bug

* Catch case for no examples
mwetter added a commit that referenced this pull request Aug 22, 2024
* Issue253 coverage (#557)

* add coverage script from Mans with changes based on review in #315 #243


---------

Co-authored-by: FWuellhorst <[email protected]>
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.

Report regressiontest coverage

2 participants