-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Refactored others type of subotimal asserts #7672
Refactored others type of subotimal asserts #7672
Conversation
Signed-off-by: Han Wang <freddie.wanah@gmail.com>
Signed-off-by: Han Wang <freddie.wanah@gmail.com>
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.
Thanks for the PR, overall looks good to me.
Only several minor comments inline.
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com> Signed-off-by: Ben Murray <ben.murray@gmail.com>
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.
Looks good with @KumoLiu's suggestions
Signed-off-by: Han Wang <freddie.wanah@gmail.com>
Thanks for the review. All prints have been removed. |
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
/build |
Hi @freddiewanah, could you please fix the flake8 error, thanks. |
Signed-off-by: Han Wang <freddie.wanah@gmail.com>
/build |
Description
As discussed in PR #7609, I tried to break the suboptimal test refactors to smaller pieces. In this PR all asserts are checking textual content or instance of a parameter.
Suboptimal Assert: Instead of using statements such as assertIsNone, assertIsInstance, always simply use assertTrue or assertFalse. This will decrease the code overall readability and increase the execution time as extra logic needed.
Types of changes
./runtests.sh -f -u --net --coverage
../runtests.sh --quick --unittests --disttests
.make html
command in thedocs/
folder.