[YUNIKORN-838] Improve coverage of allocation_ask.go

[0yukali0]

What is this PR for?

Add unit test to improve coverage

What type of PR is it?

• - Bug Fix
• - Improvement
• - Feature
• - Documentation
• - Hot Fix
• - Refactoring

Todos

• - Task

What is the Jira issue?

https://issues.apache.org/jira/browse/YUNIKORN-838

How should this be tested?

Use command "make test" to create coverage.txt and then use "go tool cover -func=coverage.txt" to check

Screenshots (if appropriate)

Questions:

• - The licenses files need update.
• - There is breaking changes for older versions.
• - It needs documentation.

[codecov]

Codecov Report

Merging #320 (9e40ef3) into master (5a1c19e) will increase coverage by 2.54%.
The diff coverage is 64.95%.

@@            Coverage Diff             @@
##           master     #320      +/-   ##
==========================================
+ Coverage   63.46%   66.00%   +2.54%     
==========================================
  Files          60       63       +3     
  Lines        5220     5937     +717     
==========================================
+ Hits         3313     3919     +606     
- Misses       1747     1814      +67     
- Partials      160      204      +44     

Impacted Files	Coverage Δ
pkg/metrics/init.go	62.96% <ø> (ø)
pkg/scheduler/drf_preemption_policy.go	0.00% <0.00%> (ø)
pkg/scheduler/nodes_usage_monitor.go	0.00% <0.00%> (ø)
pkg/scheduler/objects/allocation.go	100.00% <ø> (ø)
pkg/scheduler/objects/node_iterator.go	100.00% <ø> (ø)
pkg/scheduler/objects/sorters.go	100.00% <ø> (ø)
pkg/webservice/webservice.go	32.25% <ø> (+19.35%)	⬆️
pkg/scheduler/context.go	16.10% <23.33%> (+9.84%)	⬆️
pkg/metrics/metrics_collector.go	64.51% <45.00%> (-9.40%)	⬇️
pkg/metrics/scheduler.go	58.92% <49.33%> (-2.66%)	⬇️
... and 28 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66cdb3d...9e40ef3. Read the comment docs.

[yangwwei]

hi @0yukali0 sorry for the late response. I just looked more into this PR.
The priority actually has 2 forms, 1 is with the int value, the other is the priorityClass which is a string value, today the latter form is not fully implemented, but that's part of the proto, so it will be good to cover that as well. And regarding the UT itself, I think it can be simplified for easier reading. Below is what I thought in my mind, pls take a look:

func newPriorityValue(v int32) *si.Priority{
	return &si.Priority{
		Priority: &si.Priority_PriorityValue{
			PriorityValue: v},
	}
}

func newPriorityClass(priorityClassName string) *si.Priority {
	return &si.Priority{
		Priority: &si.Priority_PriorityClassName{
			PriorityClassName: priorityClassName},
	}
}

func TestSetPriority(t *testing.T) {
	var tests = []struct {
		testMessage           string
		priority              *si.Priority
		out                   int32
	}{
		// Set priorirty via NewAllocationAsk
		{"No priority setting",nil, 0},
		{"Set empty priority", &si.Priority{}, 0},
		{"Set positive priority value", newPriorityValue(1), 1},
		{"Set negative priority value", newPriorityValue(-100), -100},
		{"Set priority class", newPriorityClass("p0"), 0},
	}

	for _, tt := range tests {
		res := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10})
		t.Run(tt.testMessage, func(t *testing.T) {
			siAsk := &si.AllocationAsk{
				AllocationKey:  "ask-1",
				ApplicationID:  "app-1",
				MaxAllocations: 1,
				Priority:       tt.priority,
				ResourceAsk:    res.ToProto(),
			}

			ask := NewAllocationAsk(siAsk)
			if ask.priority != tt.out {
				t.Errorf("result %s, Want %v, got %v", tt.testMessage, tt.out, ask.priority)
			}
		})
	}
}

[0yukali0]

Got it.

[yangwwei]

@0yukali0 can u pls update the PR?

[0yukali0]

Got it Weiwei Yang ***@***.***> 於 2021年9月13日 週一 下午11:04 寫道：

…

@0yukali0 <https://github.com/0yukali0> can u pls update the PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#320 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AK6DJMBOC4UURWPZ6FWAWUDUBYHHFANCNFSM5DS3I6MQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[0yukali0]

Hi @yangwwei , setPriority funtion is not used in yunikorn-core now, should i add it into UT?
https://github.com/apache/incubator-yunikorn-core/blob/master/pkg/scheduler/objects/allocation_ask.go#L134-L138

[yangwwei]

+1