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

Issue253 coverage (#557) #563

Merged
merged 10 commits into from
Aug 22, 2024
Merged

Issue253 coverage (#557) #563

merged 10 commits into from
Aug 22, 2024

Conversation

mwetter
Copy link
Member

@mwetter mwetter commented Jun 21, 2024

This closes #253

* 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
* add options for startup and additional package loading
@mwetter mwetter self-assigned this Jun 21, 2024
@mwetter
Copy link
Member Author

mwetter commented Jun 26, 2024

@FWuellhorst : Can you please correct the doctests, see https://app.travis-ci.com/github/lbl-srg/BuildingsPy/builds/271076330#L695
and also update the CHANGES.txt file.

I already merged #562 into this branch to avoid a merge conflict.

@FWuellhorst FWuellhorst mentioned this pull request Jun 27, 2024
@FWuellhorst
Copy link
Contributor

@mwetter I fixed the test and added to the Changes.txt.

Another question: Using the latest buildingspy, you seemed to add a statement which prevents .mat files being copied to the tmp folder. We use .mat files for weather data in some regression tests in AixLib, leading to failing tests with no error indication, used "False". I had to use docker with port-forwarding to track down the issue. We could switch to txt or mos, but CombiTables support .mat. Why did you opt to not copy .mat files?

FWuellhorst and others added 2 commits June 28, 2024 10:42
* 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
Copy link
Member Author

mwetter commented Jun 28, 2024

@FWuellhorst : We excluded *.mat files to make sure that no old result files are copied, and to reduce time for copying/disk space if someone runs regression tests multiple times as there can be more than 1000 .mat files from old runs, some of them being quite large.
If we were to allow copying mat files in certain directories, the best may be to search for them with glob and copy them. Currently the library is copied with shutil.copytree which allows excluding files but I don't think it allows excluding files if they are in a specific directory.

@FWuellhorst
Copy link
Contributor

That makes sense. I converted them to .txt, which is either way better for git. Thanks!

@mwetter mwetter merged commit 585a47f into master Aug 22, 2024
2 checks passed
@mwetter mwetter deleted the issue253_coverage branch August 22, 2024 18:23
@mwetter
Copy link
Member Author

mwetter commented Aug 22, 2024

@FWuellhorst : FYI, this is now merged.

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