-
Notifications
You must be signed in to change notification settings - Fork 671
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
Improve resource manager test coverage #5973
Improve resource manager test coverage #5973
Conversation
Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5973 +/- ##
==========================================
+ Coverage 36.97% 37.00% +0.03%
==========================================
Files 1318 1318
Lines 132516 132516
==========================================
+ Hits 49000 49043 +43
+ Misses 79258 79229 -29
+ Partials 4258 4244 -14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Attributes: &admin.WorkflowAttributes{}, | ||
} | ||
_, failError := manager.UpdateWorkflowAttributes(context.Background(), request) | ||
assert.Error(t, failError) |
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.
Can we also make sure it returns the right error?
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.
and other similar places
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.
all new added error test cases have been error type and error message checked in the code
} | ||
|
||
_, validationError := manager.GetWorkflowAttributes(context.Background(), request) | ||
assert.Error(t, validationError) |
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.
same here
Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
…lete function error Signed-off-by: Arthur <[email protected]>
…ete function error Signed-off-by: Arthur <[email protected]>
…unction error Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
…d error message check for UpdateProjectDomainAttributes Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
…r DeleteProjectDomainAttributes Signed-off-by: Arthur <[email protected]>
Signed-off-by: Arthur <[email protected]>
90220c0
to
0372f31
Compare
Signed-off-by: Eduardo Apolinario <[email protected]>
Congrats on merging your first pull request! 🎉 |
Why are the changes needed?
the resource manager test coverage is 64.04%, which is behind 80%, this PR improves the coverage
What changes were proposed in this pull request?
add more test case coverage to resource manager in this pull request
How was this patch tested?
tests were run after every commit
Check all the applicable boxes