-
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
Merge similar test components with parameterized #7663
Merge similar test components with parameterized #7663
Conversation
Signed-off-by: Han Wang <freddie.wanah@gmail.com>
Signed-off-by: Han Wang <freddie.wanah@gmail.com>
Signed-off-by: Han Wang <freddie.wanah@gmail.com>
ef582bc
to
d01b61e
Compare
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.
Hi @freddiewanah, appreciate your contribution with the PR! Your work has indeed boosted the legibility of the testing code. I've made a few further improvement suggestions directly within the code. Thank you.
Signed-off-by: Han Wang <freddie.wanah@gmail.com>
Please take a look at the ci error looks forget name arg. Thanks. |
Signed-off-by: Han Wang <freddie.wanah@gmail.com>
Hi KumoLiu, thanks for the review, I've updated the code and now the error is fixed :) |
/build |
/build |
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 quick update!
Description
I noticed some test cases contain same duplicated asserts. Having multiple asserts in one test cases can cause potential issues like when the first assert fails, the test case stops and won't check the second assert. By using @parameterized.expand, this issue can be resolved and the caching also saves execution time.
Added sign-offs from #7648
Types of changes
./runtests.sh -f -u --net --coverage
../runtests.sh --quick --unittests --disttests
.make html
command in thedocs/
folder.