-
Notifications
You must be signed in to change notification settings - Fork 660
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
base: master
Are you sure you want to change the base?
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.86% 36.92% +0.06%
==========================================
Files 1310 1310
Lines 131246 131372 +126
==========================================
+ Hits 48380 48511 +131
+ Misses 78668 78659 -9
- Partials 4198 4202 +4
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
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