code review 7808045: image/gif: make test repeatable

image/gif: make test repeatable

Fixes issue with go test -cpu=1,1

[minux1, 2013-03-25 09:34:04]

https://codereview.appspot.com/7808045/diff/2001/src/pkg/image/gif/reader_tes...
File src/pkg/image/gif/reader_test.go (right):

https://codereview.appspot.com/7808045/diff/2001/src/pkg/image/gif/reader_tes...
src/pkg/image/gif/reader_test.go:119: copy(gif, testGIF)
is using append([]byte(nil), testGIF...) more idiomatic here?

[dfc, 2013-03-25 09:55:34]

Hello minux.ma@gmail.com (cc: golang-dev@googlegroups.com),

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

[bradfitz, 2013-03-25 15:39:28]

LGTM

On Mon, Mar 25, 2013 at 2:55 AM, <dave@cheney.net> wrote:

> Reviewers: minux,
>
> Message:
> Hello minux.ma@gmail.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> image/gif: make test repeatable
>
> Fixes issue with go test -cpu=1,1
>
> Please review this at
https://codereview.appspot.**com/7808045/<https://codereview.appspot.com/7808...
>
> Affected files:
>   M src/pkg/image/gif/reader_test.**go
>
>
> Index: src/pkg/image/gif/reader_test.**go
> ==============================**==============================**=======
> --- a/src/pkg/image/gif/reader_**test.go
> +++ b/src/pkg/image/gif/reader_**test.go
> @@ -114,22 +114,25 @@
>  }
>
>  func TestBounds(t *testing.T) {
> +       // make a local copy of testGif
> +       gif := make([]byte, len(testGIF))
> +       copy(gif, testGIF)
>         // Make the bounds too big, just by one.
> -       testGIF[32] = 2
> +       gif[32] = 2
>         want := "gif: frame bounds larger than image bounds"
> -       try(t, testGIF, want)
> +       try(t, gif, want)
>
>         // Make the bounds too small; does not trigger bounds
>         // check, but now there's too much data.
> -       testGIF[32] = 0
> +       gif[32] = 0
>         want = "gif: too much image data"
> -       try(t, testGIF, want)
> -       testGIF[32] = 1
> +       try(t, gif, want)
> +       gif[32] = 1
>
>         // Make the bounds really big, expect an error.
>         want = "gif: frame bounds larger than image bounds"
>         for i := 0; i < 4; i++ {
> -               testGIF[32+i] = 0xff
> +               gif[32+i] = 0xff
>         }
> -       try(t, testGIF, want)
> +       try(t, gif, want)
>  }
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[nigeltao, 2013-03-26 01:23:19]

LGTM.

https://codereview.appspot.com/7808045/diff/9001/src/pkg/image/gif/reader_tes...
File src/pkg/image/gif/reader_test.go (right):

https://codereview.appspot.com/7808045/diff/9001/src/pkg/image/gif/reader_tes...
src/pkg/image/gif/reader_test.go:117: // make a local copy of testGif
// Make a local copy of testGIF.

[dfc, 2013-03-26 03:17:10]

Thanks Nigel, I will address that comment before I submit. 

On 26/03/2013, at 12:23, nigeltao@golang.org wrote:

> LGTM.
> 
> 
>
https://codereview.appspot.com/7808045/diff/9001/src/pkg/image/gif/reader_tes...
> File src/pkg/image/gif/reader_test.go (right):
> 
>
https://codereview.appspot.com/7808045/diff/9001/src/pkg/image/gif/reader_tes...
> src/pkg/image/gif/reader_test.go:117: // make a local copy of testGif
> // Make a local copy of testGIF.
> 
> https://codereview.appspot.com/7808045/

[dfc, 2013-03-26 05:20:29]

*** Submitted as https://code.google.com/p/go/source/detail?r=f3b1ca87b5c4 ***

image/gif: make test repeatable

Fixes issue with go test -cpu=1,1

R=minux.ma, bradfitz, nigeltao
CC=golang-dev
https://codereview.appspot.com/7808045

https://codereview.appspot.com/7808045/diff/2001/src/pkg/image/gif/reader_tes...
File src/pkg/image/gif/reader_test.go (right):

https://codereview.appspot.com/7808045/diff/2001/src/pkg/image/gif/reader_tes...
src/pkg/image/gif/reader_test.go:119: copy(gif, testGIF)
On 2013/03/25 09:34:04, minux wrote:
> is using append([]byte(nil), testGIF...) more idiomatic here?

My personal feeling is it is not, I dislike using this append side effect. I am
happy to be overruled on this matter.