SimonSimonB opened a new pull request, #554: URL: https://github.com/apache/yunikorn-core/pull/554
### What is this PR for? [YUNIKORN-1403](https://issues.apache.org/jira/browse/YUNIKORN-1403) asks for a test so that [this code](https://github.com/apache/yunikorn-core/blob/master/pkg/common/resources/resources.go#L965-L972) handling a negative overflow, which is currently not covered by the tests, is tested. In fact, there already exists [a test](https://github.com/apache/yunikorn-core/blob/master/pkg/common/resources/resources_test.go#L1512-L1516) that is meant to exercise that code. As far as I understand, the reason that this test does not exercise the code is that the [type conversion](https://github.com/apache/yunikorn-core/blob/master/pkg/common/resources/resources.go#L956) from `float64` to `int64` does not lead to negative overflows (at least not for the values that I tried!), although it does lead to positive overflows. This is demonstrated by the following Go program: ```go package main import ( "fmt" "math" ) func main() { tooSmall := float64(math.MinInt64) * 100 tooBig := float64(math.MaxInt64) * 100 fmt.Printf("tooSmall < math.MinInt64: %t\n", tooSmall < math.MinInt64) fmt.Printf("int64(tooSmall) > 0: %t\n", int64(tooSmall) > 0) fmt.Printf("tooBig > math.MaxInt64: %t\n", tooBig > math.MaxInt64) fmt.Printf("int64(tooBig) < 0: %t\n", int64(tooBig) < 0) } ``` which prints ``` tooSmall < math.MinInt64: true int64(tooSmall) > 0: false tooBig > math.MaxInt64: true int64(tooBig) < 0: true ``` in Go 1.20. This behaviour does not violate the [Go spec](https://go.dev/ref/spec#Conversions), which leaves the behaviour in this case undefined: > In all non-constant conversions involving floating-point or complex values, if the result type cannot represent the value the conversion succeeds but the result value is implementation-dependent. So, the test currently does not exercise the code it wants to exercise, but it may do so in future versions of Go. My sense is that it would be best to 1) keep the test in case future implementations lead to negative overflows, 2) add a comment above the uncovered code to explain why tests may not exercise this code in some versions of Go. This PR adds the explanatory comment. (Someone else self-assigned the JIRA issue back in November. Since this is a small issue and no activity was recorded since November, I assumed it would be fine to open a PR addressing it. I hope that was okay.) ### What type of PR is it? * [ ] - Bug Fix * [ ] - Improvement * [ ] - Feature * [X] - Documentation * [ ] - Hot Fix * [ ] - Refactoring ### Todos ### What is the Jira issue? [YUNIKORN-1403](https://issues.apache.org/jira/browse/YUNIKORN-1403) ### How should this be tested? ### Screenshots (if appropriate) ### Questions: * [ ] - The licenses files need update. * [ ] - There is breaking changes for older versions. * [ ] - It needs documentation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
