-
Notifications
You must be signed in to change notification settings - Fork 1.5k
refs #12232/#7772 - added script to detect missing --errorlist entries and test coverage / added some missing --errorlist entries
#7439
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
Conversation
|
Requires the error ID to be added to the expected output of all unit tests to be fully effective. |
b8cc289 to
1b83028
Compare
59eb818 to
021b7ec
Compare
ab2fe8c to
c886e8e
Compare
--errorlist entries and test coverage--errorlist entries and test coverage
af122b0 to
791a104
Compare
--errorlist entries and test coverage--errorlist entries and test coverage / added some missing --errorlist entries
079a40c to
c0c70e1
Compare
--errorlist entries and test coverage / added some missing --errorlist entries--errorlist entries and test coverage / added some missing --errorlist entries
|
CC @amai2012 |
|
|
||
| #set -x | ||
|
|
||
| SCRIPT_DIR="$(dirname "$(realpath "$0")")" |
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.
Imho it would be easier to maintain a python script than a bash script.
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.
Yes, but I started out with these shell commands and I didn't feel like try to replicate it in Python for now. It is also sufficient until we want to integrate it into the CI.
| @@ -0,0 +1,31 @@ | |||
| #!/bin/bash | |||
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.
the filename is pretty generic so it's not easy to understand exactly what it does. And there is no comment that explains it neither. can it be clarified..
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.
Agreed. Will adjust the name and document it in the related markdown file.
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 added some documentation in the readme.md of the tools folder.
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 still think the filename is too generic but I don't feel I have a very good suggestion..
how about:
- compare-errorlist-testsuite
- get-missing-ids-errorlist-testsuite
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 tried coming up with something but couldn't. The suggestions are equally missing the point.
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 will give it another thought tomorrow as I am also not satisfied with the name.
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.
- compare-errorids
- check-errorids
- check-errorid-coverage
Definitely something containing "errorid".
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 don't like "check-errorids" . but no strong opinion about those other. they are better than the existing filename.
imho I would add testsuite-errorlist into your names.
- compare-errorids-testsuite-errorlist
- check-errorid-coverage-testsuite-errorlist
the names are a bit longish but imho this is fine.
|



No description provided.