-
Notifications
You must be signed in to change notification settings - Fork 358
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
Refactor the lifecycle test #1868
Conversation
Signed-off-by: yihuaf <[email protected]>
Signed-off-by: yihuaf <[email protected]>
Signed-off-by: yihuaf <[email protected]>
Signed-off-by: yihuaf <[email protected]>
Signed-off-by: yihuaf <[email protected]>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1868 +/- ##
=======================================
Coverage 68.89% 68.89%
=======================================
Files 122 122
Lines 13717 13717
=======================================
+ Hits 9450 9451 +1
+ Misses 4267 4266 -1 |
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.
Hey, overall its fine, just a few nits.
As you had said this makes much more sense that what we currently have. Thanks for taking initiative and making this change!
tests/rust-integration-tests/integration_test/src/tests/lifecycle/container_create.rs
Outdated
Show resolved
Hide resolved
tests/rust-integration-tests/integration_test/src/tests/lifecycle/container_create.rs
Outdated
Show resolved
Hide resolved
68eb0c3
to
2843bf9
Compare
@YJDoc2 PTAL |
Signed-off-by: yihuaf <[email protected]>
2843bf9
to
148f194
Compare
Signed-off-by: yihuaf <[email protected]>
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.
lgtm 👍
As previously discussed in #1827 , the
TestResult
is meant to be used in the test cases and not utility functions. Before this PR, the lifecycle test calls other tests usingTestResult
which is not inline with our intention. This PR fix this by refactoring all tests to not call other tests in test cases.