-
Notifications
You must be signed in to change notification settings - Fork 41
Attribute coverage script #403
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
Attribute coverage script #403
Conversation
GabrielSoto-INL
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can confirm that it takes a while to complete the attribute_coverage script run... Ran it locally in about 2 hours. The new context feature is very cool, really useful addition to the coverage tests. It feels like this is the final step to provide a complete coverage report.
I was wondering though if the scripts could:
- accept the
-jargument and feed it to rook which allows multiple tests to run simultaneously - run heavy tests as well, using the
--heavyargument
Do you think this could be implemented? If so, do you think it could be included in this PR or might be best as a separate PR?
|
|
||
| coverage erase | ||
| ($RAVEN_DIR/run_tests "$@" --re=HERON/tests --python-command="coverage run $EXTRA ") | ||
| ($RAVEN_DIR/run_tests "${ARGS[@]}" --re="$REGEX" --python-command="coverage run $COV_CLARGS $EXTRA ") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if additional arguments could be added here for some additional rook args (e.g., --j to specify the number of simultaneous/parallel jobs which might speed up the tests or --heavy to also include heavy tests).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GabrielSoto-INL When the command line arguments to check_py_coverage are read in, any unrecognized arguments are placed in the ARGS array, which is expanded and provided as a string of command line arguments to run_tests in line 54. Effectively, any arguments that are not recognized by check_py_coverage are fed unaltered into run_tests. This is the same way that run_tests handles command line arguments intended for rook. I've specifically gotten in the middle of this process for the --re argument in order to create a default if no value is provided by the user, but the rest should just work like they would if put into run_tests directly.
|
@GabrielSoto-INL I could easily modify |
|
I think that might be worth it, at the very least to speed things up. would also allow the option to run the heavy tests as well |
GabrielSoto-INL
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, with that last change I was able to locally run the attribute coverage script with an additional -f4 command. I did some very crude timings, and it improves the computation time:
attribute_coverage.sh |
attribute_coverage.sh -f4 |
|---|---|
| 156m 47.616s | 58m 9.033s |
Changes LGTM, good to merge
Pull Request Description
What issue does this change request address?
#402
What are the significant changes in functionality due to this change request?
The main addition is a script that iterates through a list of tests, runs coverage with a regex filter for each individual test, and marks each run with a separate "context", which is shown in the report generated. This script is, admittedly, painfully slow and inefficient, completing a full
check_py_coveragerun for every test; however, the information it seeks to obtain is, on average, unlikely to change greatly in the short term. The current output of this script is the following report:coverage_attribution_report.zip (To access report: download, unzip, and open "index" file)
Some changes were also made to the
check_py_coverage.shscript. These mainly consisted of command line arguments to supportattribute_coverageand debugging to deal properly with Windows filepaths.Note:
attribute_coverageis dependent onraven.developer_utils.get_coverage_tests.py. This script was updated with https://github.com/idaholab/raven/pull/2431 to supportattribute_coverage. Future faulty output fromattribute_coveragecould be due toget_coverage_testsbecoming outdated.For Change Control Board: Change Request Review
The following review must be completed by an authorized member of the Change Control Board.