code review 4004044: test: another select test

test: another select test

[rsc, 2011-01-29 05:23:40]

Hello r (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[r, 2011-01-29 05:39:49]

shouldn't there be stuff to run this?

[rsc, 2011-01-29 14:58:31]

On Sat, Jan 29, 2011 at 00:39, Rob 'Commander' Pike <r@golang.org> wrote:
> shouldn't there be stuff to run this?

the script at the the top of the file was copied from
test/64bit.go: generate to tmp.go, compile, run.

[r, 2011-01-29 19:22:41]

LGTM

i notice that this test uses a buffered channel and therefore cannot test the
bug that led to the test being created.  that's not a criticism of the test but
an indication that another is probably needed.

interesting approach.

http://codereview.appspot.com/4004044/diff/5001/test/chan/select5.go
File test/chan/select5.go (right):

http://codereview.appspot.com/4004044/diff/5001/test/chan/select5.go#newcode2
test/chan/select5.go:2: // $G tmp.go && $L tmp.$A && ./$A.out || echo BUG:
select6
s/6/5/?

http://codereview.appspot.com/4004044/diff/5001/test/chan/select5.go#newcode140
test/chan/select5.go:140: t.SetDelims("〈", "〉")
this is cute and i'm not suggesting you change it but you could just have done
t.SetDelims("$", "$")
and why the spaces?

http://codereview.appspot.com/4004044/diff/5001/test/chan/select5.go#newcode148
test/chan/select5.go:148: 〈# Send n, receive it one way or another into x, check
that they match.〉
why aren't these Go comments? on failure, it might be nice to have them in the
generated code.

[rsc, 2011-01-30 17:13:45]

> i notice that this test uses a buffered channel and therefore cannot
> test the bug that led to the test being created.  that's not a criticism
> of the test but an indication that another is probably needed.

This test was motivated by writing code in the compiler to replace
1-case and 1-case+default selects with special purpose code, as
promised in the proposal for eliminating non-blocking channel syntax.
It found a lot of bugs in that code (not yet checked in, different CL).

I agree about a more exhaustive test of select's blocking behavior, though.

Russ

[rsc, 2011-01-30 20:32:43]

http://codereview.appspot.com/4004044/diff/5001/test/chan/select5.go
File test/chan/select5.go (right):

http://codereview.appspot.com/4004044/diff/5001/test/chan/select5.go#newcode140
test/chan/select5.go:140: t.SetDelims("〈", "〉")
On 2011/01/29 19:22:41, r wrote:
> this is cute and i'm not suggesting you change it but you could just have done
> t.SetDelims("$", "$")
> and why the spaces?

I chose 〈〉 because they are particularly light
on the page in acme (and in Linux Chrome, fwiw).
I started with « and » but they made it hard to see
the code for the comments.  The "spaces" are OS X
Chrome/Safari not displaying things right.
The delims are single-rune strings.

http://codereview.appspot.com/4004044/diff/5001/test/chan/select5.go#newcode148
test/chan/select5.go:148: 〈# Send n, receive it one way or another into x, check
that they match.〉
On 2011/01/29 19:22:41, r wrote:
> why aren't these Go comments? on failure, it might be nice to have them in the
> generated code.

Most of the comments are about testing strategy:
test with and without this case because the compiler
handles those two differently.  There are only tens
of comments here, but 1000s of generated tests,
which implies the comments couldn't be that helpful
to any specific test beyond echoing what the code does.

[rsc, 2011-01-30 20:46:11]

*** Submitted as 1777269ed52f ***

test: another select test

R=r
CC=golang-dev
http://codereview.appspot.com/4004044