Add unit tests for queuejob util functions #545
Conversation
ChristianZaccaria
requested review from
z103cb
and removed request for
astefanutti and
dimakis
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| result := HigherSystemPriorityQJ(tt.qj1, tt.qj2) |
There was a problem hiding this comment.
Can you change the method signature to not use interface{} and use AppWrapper ?
There was a problem hiding this comment.
I added a new commit to make use of AppWrapper, it required me to change the heap.go functions as well to use the AppWrapper type. I'm not sure if this is the ideal way of doing it, hoping to get your thoughts/advice on the changes made. Thank you @z103cb
There was a problem hiding this comment.
They are LGTM. @astefanutti please review this change and approve it. I think it might be conflict with your k8s upgrade PR>
|
/lgtm Thanks! |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: astefanutti The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Issue link
Closes #544
What changes have been made
Created unit tests on the functions in queuejob util, covering different scenarios.
Verification steps
Tests can be run locally and see them passing.
Checks