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

Comments

Projects
None yet
4 participants
@kr
Copy link
Contributor

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

commented May 31, 2013

Comment 2:

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

This comment has been minimized.

Copy link
Contributor

commented May 31, 2013

Comment 3:

This issue was updated by revision 787976c.

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2013

Comment 5:

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

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2013

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.
You can’t perform that action at this time.