Repository navigation
Refactor test cases to improve unit test quality - #3298
Conversation
|
The assertIsNone and assertIsNotNone changes are good and I'd be happy to see those methods adopted wherever applicable. (The same is true in most cases for all of the new assertion methods that were introduced in python 3, although sometimes it's more subtle and it's not quite equivalent to switch) For the changes in gen_test that use str.join, though, I prefer them as they are. It's easier to read the single string than the list of string literals. |
|
Sorry for the late reply. I did more static analysis to the code base during the time and got time to refactor more test cases. In addition, I fixed more inconsistencies in the code, e.g., using assertIn and assertIsNone instead of keep using assertTrue and assertFalse. I found that using more precise assert statement could save the execution time of each individual test methods and hence save the overall project cost during the CICD. |
|
@bdarnell May I get some updates on this one? |
|
Thanks, all these changes look great. I'm not expecting to see any savings in execution time and CI costs, but the new assertion methods give much better error messages when they fail so this is a worthwhile change. We do have a few |
Signed-off-by: Han Wang <[email protected]>
|
Thanks for the review, and I've pushed a new commit to fix the format issue. Sorry about the previous check failed. |
|
Thanks! |
That PR arrived while our CI was broken and I manually verified that the tests passed but didn't run the linters. These changes are running in to python/mypy#5088
test: Fix some lint issues after #3298
Hello, first-time submitting PR here.
This PR improves the readability of the assert statements in some of the unit tests by using more expressive assert methods from the unittest module. These changes improve code quality (avoid test smells) by:
Happy to update more related issues in the test code if needed :)