-
Notifications
You must be signed in to change notification settings - Fork 139
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
[YUNIKORN-2540] Deduplicate constants in pkg/cache/context_test.go #866
Conversation
…D, APIVersion1, testUser, and so on.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #866 +/- ##
==========================================
- Coverage 67.38% 67.30% -0.09%
==========================================
Files 70 70
Lines 7623 7625 +2
==========================================
- Hits 5137 5132 -5
- Misses 2268 2274 +6
- Partials 218 219 +1 ☔ View full report in Codecov by Sentry. |
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.
A minor suggestion.
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.
Simplifications are possible and some consistency-related stuff. See comments.
@SophieTech88 please rebase your branch with master changes. |
Actually never mind, I did myself so we can have a green build. |
Many thanks for your help. |
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 @SophieTech88 for the update. A few minor comments.
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.
+1 LGTM, will merge it once E2E test pass.
What is this PR for?
Constants are duplicated in the pkg/cache/context_test.go
Move to a central point of defining the constants for the test at the top of the file.
The related constant variables as below:
const (
)
What type of PR is it?
Todos
What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-2540
How should this be tested?
It should pass all test cases.
Screenshots (if appropriate)
Questions: