Skip to content
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

testing: roundDown10 sometimes rounds too far #5599

Closed
kr opened this issue May 31, 2013 · 6 comments
Closed

testing: roundDown10 sometimes rounds too far #5599

kr opened this issue May 31, 2013 · 6 comments

Comments

@kr
Copy link
Contributor

kr commented May 31, 2013

Run http://play.golang.org/p/kX3sgrx7c4

This calls roundDown10 (copied verbatim from package testing)
with 10001 as input.

What is the expected output?

10000

What do you see instead?

1000

Solution:

Test n > 10 should be n >= 10.

This is the least consequential bug I can ever remember reporting.

I'll submit a code review if no one beats me to it.
@minux
Copy link
Member

minux commented May 31, 2013

Comment 1:

not a big deal, but ok, please send the CL.

Labels changed: added priority-someday, removed priority-triage.

Status changed to Accepted.

@davecheney
Copy link
Contributor

Comment 2:

https://golang.org/cl/9738052 adds a test which shows the issue (commented out).

@davecheney
Copy link
Contributor

Comment 3:

This issue was updated by revision 787976c.

R=golang-dev, r, minux.ma
CC=golang-dev
https://golang.org/cl/9738052

@minux
Copy link
Member

minux commented May 31, 2013

Comment 4:

I suggest this version:
func roundDown10(n int) int {
        ret := 1
        for ret < n && ret * 10 < n {
                ret *= 10
        }
        return ret
}
ps: also this patch:
diff -r fd791d51e476 src/pkg/testing/benchmark_test.go
--- a/src/pkg/testing/benchmark_test.go Fri May 31 23:03:22 2013 +1000
+++ b/src/pkg/testing/benchmark_test.go Fri May 31 21:27:16 2013 +0800
@@ -16,16 +16,16 @@
        {10, 1},
        {11, 10},
        {100, 10},
-       //      {101, 100}, // issue #5599
+       {101, 100}, // issue #5599
        {1000, 100},
-       //      {1001, 1000}, // issue #5599
+       {1001, 1000}, // issue #5599
 }
 func TestRoundDown10(t *testing.T) {
        for _, tt := range roundDownTests {
                actual := testing.RoundDown10(tt.v)
                if tt.expected != actual {
-                       t.Errorf("roundDown10: expected %v, actual %v", tt.expected,
actual)
+                       t.Errorf("roundDown10(%v): expected %v, actual %v", tt.v,
tt.expected, actual)
                }
        }
 }
i failed to submit the comment in time for CL 9738052.

@kr
Copy link
Contributor Author

kr commented Jun 1, 2013

Comment 5:

Just sent https://golang.org/cl/9915045
s/least consequential/most important/
Apologies for the typographical error.

@davecheney
Copy link
Contributor

Comment 6:

This issue was closed by revision a28609d.

Status changed to Fixed.

@kr kr added fixed labels Jun 1, 2013
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants