code review 5542055: fmt: enable and fix malloc test

fmt: enable and fix malloc test
On 32-bit machines, %g takes an extra malloc. I don't know why yet,
but this makes the test pass again, and enables it even for -short.

Fixes issue 2653.

[r, 2012-01-17 18:37:53]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2012-01-17 18:43:30]

LGTM

Add a TODO on that test line or file a bug about the 2-vs-1 on 32-bit, so
it's not forgotten?


On Tue, Jan 17, 2012 at 10:37 AM, <r@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> fmt: enable and fix malloc test
> On 32-bit machines, %g takes an extra malloc. I don't know why yet,
> but this makes the test pass again, and enables it even for -short.
>
> Fixes issue 2653.
>
> Please review this at
http://codereview.appspot.com/**5542055/<http://codereview.appspot.com/5542055/>
>
> Affected files:
>  M src/pkg/fmt/fmt_test.go
>
>
> Index: src/pkg/fmt/fmt_test.go
> ==============================**==============================**=======
> --- a/src/pkg/fmt/fmt_test.go
> +++ b/src/pkg/fmt/fmt_test.go
> @@ -509,16 +509,16 @@
>  var mallocBuf bytes.Buffer
>
>  var mallocTest = []struct {
> -       count int
> -       desc  string
> -       fn    func()
> +       max  int
> +       desc string
> +       fn   func()
>  }{
>        {0, `Sprintf("")`, func() { Sprintf("") }},
>        {1, `Sprintf("xxx")`, func() { Sprintf("xxx") }},
>        {1, `Sprintf("%x")`, func() { Sprintf("%x", 7) }},
>        {2, `Sprintf("%s")`, func() { Sprintf("%s", "hello") }},
>        {1, `Sprintf("%x %x")`, func() { Sprintf("%x %x", 7, 112) }},
> -       {1, `Sprintf("%g")`, func() { Sprintf("%g", 3.14159) }},
> +       {2, `Sprintf("%g")`, func() { Sprintf("%g", 3.14159) }},
>        {0, `Fprintf(buf, "%x %x %x")`, func() { mallocBuf.Reset();
> Fprintf(&mallocBuf, "%x %x %x", 7, 8, 9) }},
>        {1, `Fprintf(buf, "%s")`, func() { mallocBuf.Reset();
> Fprintf(&mallocBuf, "%s", "hello") }},
>  }
> @@ -526,9 +526,6 @@
>  var _ bytes.Buffer
>
>  func TestCountMallocs(t *testing.T) {
> -       if testing.Short() {
> -               return
> -       }
>        for _, mt := range mallocTest {
>                const N = 100
>                runtime.UpdateMemStats()
> @@ -538,8 +535,8 @@
>                }
>                runtime.UpdateMemStats()
>                mallocs += runtime.MemStats.Mallocs
> -               if mallocs/N != uint64(mt.count) {
> -                       t.Errorf("%s: expected %d mallocs, got %d",
> mt.desc, mt.count, mallocs/N)
> +               if mallocs/N > uint64(mt.max) {
> +                       t.Errorf("%s: expected at most %d mallocs, got
> %d", mt.desc, mt.max, mallocs/N)
>                }
>        }
>  }
>
>
>

[r2, 2012-01-17 18:44:31]

On Jan 17, 2012, at 10:43 AM, Brad Fitzpatrick wrote:

> LGTM
> 
> Add a TODO on that test line or file a bug about the 2-vs-1 on 32-bit, so it's
not forgotten?

i already created an issue, but sure.

-rob

[r, 2012-01-17 18:45:38]

*** Submitted as 7e995741e6cb ***

fmt: enable and fix malloc test
On 32-bit machines, %g takes an extra malloc. I don't know why yet,
but this makes the test pass again, and enables it even for -short.

Fixes issue 2653.

R=golang-dev, bradfitz, r
CC=golang-dev
http://codereview.appspot.com/5542055

[dave_cheney.net, 2012-01-17 19:41:01]

Thanks Rob. 


On 18/01/2012, at 5:44, Rob 'Commander' Pike <r@google.com> wrote:

> 
> On Jan 17, 2012, at 10:43 AM, Brad Fitzpatrick wrote:
> 
>> LGTM
>> 
>> Add a TODO on that test line or file a bug about the 2-vs-1 on 32-bit, so
it's not forgotten?
> 
> i already created an issue, but sure.
> 
> -rob
> 
>