Hk improve tests #360
Merged
Hk improve tests #360
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When forgetting to install the protobuf compiler, an error message is shown which had a missing space between two words.
There is no reason to traverse the file line by line if all we care about is the last character. This also uses unittests subTest to show the file which fails the test.
When one file is failing, due to the fact that all tests contain for loops, the other failures are swallowed because python stops running the test on the first assertion. With subTest one can introduce a subcontext which allows showing all of the failed files as well a diagnostic as to which file failed.
Python provides mechanisms for that, no reason to manually track a count.
The globs where repeated a whole bunch of times which is both information duplication as well as unneccessary work.
There is no reason to guard the assertions in the conditional.
hanneskaeufler
commented
Dec 13, 2019
| for file in DOC_FILES: | ||
| with open(file, "rt") as fin, self.subTest(file=file): | ||
| for i, line in enumerate(fin, start=1): | ||
| self.assertNotRegex(line, r"([\s>]|^)#\w(\S)*", file + " in line " + str(i) + ": not permitted hash found.") |
Contributor
Author
There was a problem hiding this comment.
FAIL: test_hash (test_doxygen_output.TestDoxygenOutput) (file='doc/html/windows.html')
Test case is checking if there are illegal hash chars in the documentation. -> doxygen link not found.
----------------------------------------------------------------------
Traceback (most recent call last):
File "open-simulation-interface/tests/test_doxygen_output.py", line 17, in test_hash
self.assertNotRegex(line, r"([\s>]|^)#\w(\S)*", file + " in line " + str(i) + ": not permitted hash found.")
AssertionError: Regex matched: '#lol' matches '([\\s>]|^)#\\w(\\S)*' in '#lol\n' : doc/html/windows.html in line 5: not permitted hash found.
I think the assertion message is good enough so that we dont need to manually add the offending section again.
hanneskaeufler
force-pushed
the
hk-improve-tests
branch
2 times, most recently
from
63dfbd2 to
1a0dc73
Compare
Python2 is end of life in just a few days and should not be recommended anymore.
The error message is quite good when using the provided regex matcher.
hanneskaeufler
force-pushed
the
hk-improve-tests
branch
from
1a0dc73 to
627c7a9
Compare
hanneskaeufler
commented
Dec 13, 2019
| on: | ||
| branch: master | ||
| on: | ||
| branch: master |
Contributor
Author
There was a problem hiding this comment.
Seems like this was indented incorrectly which caused PRs to deploy the docs :)
Contributor
There was a problem hiding this comment.
I think we have a new maintainer 💃
Contributor
There was a problem hiding this comment.
True thanks for finding :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference to a related issue in the repository
None - These are out of context quality of life improvements in the automated test suite
This PR contains #359 except from the branch pushed to the osi organization instead of to my fork. Maybe travis isn't configured to build fork PRs (I think this is the default due to potentially leaking env secrets into forks.)
Add a description
I saw some long-hanging fruit in improving these tests. I hope the commit messages do
well enough to explain the change/intention behind it. If not, feel free to comment on details :)
This PR switches the build to python3. Python2 is EOL in just a few days so I think it is reasonable to makes this switch (let me know if there are reasons not to do this!).
Check the checklist