Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(813)

Issue 150980043: code review 150980043: cmd/go: always build _test.go files and link into test (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by rsc
Modified:
9 years, 7 months ago
Reviewers:
iant
CC:
golang-codereviews, iant, adg, r
Visibility:
Public.

Description

cmd/go: always build _test.go files and link into test go test's handling of _test.go files when the entire package's set of files has no Test functions has varied over the past few releases. There are a few interesting cases (all contain no Test functions): (1) x_test.go has syntax errors (2) x_test.go has type errors (3) x_test.go has runtime errors (say, a func init that panics) In Go 1.1, tests with (1) or (2) failed; (3) passed. In Go 1.2, tests with (1) or (2) failed; (3) passed. In Go 1.3, tests with (1) failed; (2) or (3) passed. After this CL, tests with (1), (2), or (3) all fail. This is clearly a corner case, but it seems to me that the behavior of the test should not change if you add or remove a line like func TestAlwaysPasses(t *testing.T) {} That implies that the _test.go files must always be built and always be imported into the test binary. Doing so means that (1), (2), and (3) must all fail. Fixes issue 8337.

Patch Set 1 #

Patch Set 2 : diff -r d62cdebe7513197cbf70b87bbade7154d4359a55 https://code.google.com/p/go/ #

Patch Set 3 : diff -r d62cdebe7513197cbf70b87bbade7154d4359a55 https://code.google.com/p/go/ #

Patch Set 4 : diff -r 6592b0fb7a5db6b3351cfae6db876f155d626eef https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -2 lines) Patch
M src/cmd/go/test.bash View 1 1 chunk +14 lines, -0 lines 0 comments Download
M src/cmd/go/test.go View 1 1 chunk +4 lines, -2 lines 0 comments Download
A src/cmd/go/testdata/src/badtest/badexec/x_test.go View 1 1 chunk +5 lines, -0 lines 0 comments Download
A src/cmd/go/testdata/src/badtest/badsyntax/x.go View 1 1 chunk +1 line, -0 lines 0 comments Download
A src/cmd/go/testdata/src/badtest/badsyntax/x_test.go View 1 1 chunk +3 lines, -0 lines 0 comments Download
A src/cmd/go/testdata/src/badtest/badvar/x.go View 1 1 chunk +1 line, -0 lines 0 comments Download
A src/cmd/go/testdata/src/badtest/badvar/x_test.go View 1 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 3
rsc
Hello golang-codereviews@googlegroups.com (cc: adg, iant, r), I'd like you to review this change to https://code.google.com/p/go/
9 years, 7 months ago (2014-09-26 19:31:13 UTC) #1
iant
LGTM Needs to be mentioned in go1.4.txt.
9 years, 7 months ago (2014-09-26 20:20:20 UTC) #2
rsc
9 years, 7 months ago (2014-09-26 21:09:16 UTC) #3
*** Submitted as https://code.google.com/p/go/source/detail?r=c47d4e23f986 ***

cmd/go: always build _test.go files and link into test

go test's handling of _test.go files when the entire
package's set of files has no Test functions has varied
over the past few releases. There are a few interesting
cases (all contain no Test functions):
        (1) x_test.go has syntax errors
        (2) x_test.go has type errors
        (3) x_test.go has runtime errors (say, a func init that panics)

In Go 1.1, tests with (1) or (2) failed; (3) passed.
In Go 1.2, tests with (1) or (2) failed; (3) passed.
In Go 1.3, tests with (1) failed; (2) or (3) passed.
After this CL, tests with (1), (2), or (3) all fail.

This is clearly a corner case, but it seems to me that
the behavior of the test should not change if you
add or remove a line like

        func TestAlwaysPasses(t *testing.T) {}

That implies that the _test.go files must always
be built and always be imported into the test binary.
Doing so means that (1), (2), and (3) must all fail.

Fixes issue 8337.

LGTM=iant
R=golang-codereviews, iant
CC=adg, golang-codereviews, r
https://codereview.appspot.com/150980043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b