code review 6976050: bytes: fix UnreadByte failure after ReadBytes

bytes: fix UnreadByte failure after ReadBytes

Fixes issue 4583.

[stephane.travostino, 2012-12-22 17:42:14]

Hello golang-dev@googlegroups.com,

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

[minux1, 2012-12-22 18:49:19]

in the description, please use something like
bytes: *fix* UnreadByte failure after ReadByte

https://codereview.appspot.com/6976050/diff/3003/src/pkg/bytes/buffer_test.go
File src/pkg/bytes/buffer_test.go (right):

https://codereview.appspot.com/6976050/diff/3003/src/pkg/bytes/buffer_test.go...
src/pkg/bytes/buffer_test.go:457: func TestUnread(t *testing.T) {
i suggest TestUnreadByteAfterReadBytes or something similar.

[bradfitz, 2012-12-22 18:52:14]

nits...

https://codereview.appspot.com/6976050/diff/3003/src/pkg/bytes/buffer_test.go
File src/pkg/bytes/buffer_test.go (right):

https://codereview.appspot.com/6976050/diff/3003/src/pkg/bytes/buffer_test.go...
src/pkg/bytes/buffer_test.go:463: t.Errorf("ReadBytes error: %v", err)
Fatalf, as the test can't really be expected to yield any useful results after
if this fails.

you could also drop the word "error" from the format string, as the test failing
is obviously an error.

https://codereview.appspot.com/6976050/diff/3003/src/pkg/bytes/buffer_test.go...
src/pkg/bytes/buffer_test.go:475: t.Errorf("got '%c', expected '%c'", c, 'm')
you can use %q instead of '%c'

"ReadByte = %q; want %q", c, 'm'

[stephane.travostino, 2012-12-23 16:59:19]

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

Please take another look.

[minux1, 2012-12-23 17:07:00]

https://codereview.appspot.com/6976050/diff/3003/src/pkg/bytes/buffer_test.go
File src/pkg/bytes/buffer_test.go (right):

https://codereview.appspot.com/6976050/diff/3003/src/pkg/bytes/buffer_test.go...
src/pkg/bytes/buffer_test.go:457: func TestUnread(t *testing.T) {
On 2012/12/22 18:49:19, minux wrote:
> i suggest TestUnreadByteAfterReadBytes or something similar.
I still think TestUnreadByte is too board as this only tests
a specific usage sequence. IMO, if you name this function
TestUnreadByte, you should test all user visible behavior
of UnreadByte.

https://codereview.appspot.com/6976050/diff/2002/src/pkg/bytes/buffer_test.go
File src/pkg/bytes/buffer_test.go (right):

https://codereview.appspot.com/6976050/diff/2002/src/pkg/bytes/buffer_test.go...
src/pkg/bytes/buffer_test.go:468: t.Errorf("UnreadByte error: %v", err)
this should also be t.Fatalf("UnreadByte: %v", err)
as if this UnreadByte() fails, the test has no reason to continue.

https://codereview.appspot.com/6976050/diff/2002/src/pkg/bytes/buffer_test.go...
src/pkg/bytes/buffer_test.go:472: t.Errorf("ReadByte error: %v", err)
ditto, t.Fatalf("ReadByte: %v", err)

[rsc, 2013-01-02 22:40:36]

LGTM

TestUnreadbyte is fine. Please do make the Fatalf changes minux suggested.

[stephane.travostino, 2013-01-07 13:10:33]

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

Please take another look.

[dave_cheney.net, 2013-01-09 21:55:06]

I will submit this today unless there are any remaining comments. Minux ?

[dave_cheney.net, 2013-01-11 06:02:37]

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

bytes: fix UnreadByte failure after ReadBytes

Fixes issue 4583.

R=golang-dev, minux.ma, bradfitz, rsc, dave
CC=golang-dev
https://codereview.appspot.com/6976050

Committer: Dave Cheney <dave@cheney.net>

[remyoudompheng, 2013-07-20 20:08:27]

R=close

[stephane.travostino, 2013-08-14 09:57:42]

*** Abandoned ***