Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

testing: 'go test' broken in go1.13 #34443

Closed
gbbr opened this issue Sep 21, 2019 · 6 comments
Closed

testing: 'go test' broken in go1.13 #34443

gbbr opened this issue Sep 21, 2019 · 6 comments
Labels
FrozenDueToAge

Comments

@gbbr
Copy link
Member

gbbr commented Sep 21, 2019 •
edited

With go1.13 the go test command broke due to the fix for #21051.

Consider the following program made of two files:

  • file.go:
package main

import (
	"flag"
	"fmt"
)

var x string

func init() {
	flag.StringVar(&x, "x", "y", "sets a value for x")
	flag.Parse()
}

func getNumber() int { return 2 }

func main() {
	fmt.Println("Hello world")
}
  • file_test.go:
package main

import "testing"

func TestGetNumber(t *testing.T) {
	if getNumber() != 2 {
		t.Fatal("did not get 2")
	}
}

Building the test binary fails to attach the test flags, resulting in an error of the type:

flag provided but not defined: -test.v
Usage of /var/folders/02/23s7r95n6t1739kdh0zc3trr0000gn/T/go-build752851519/b001/go1.13-flagbug.test:
  -x string
    	sets a value for x (default "y")
FAIL	_/Users/gabriel.aszalos/g/go1.13-flagbug	0.005s
FAIL

Consider the output of the below command with go1.13:

$ go test -o ./file.test -c && ./file.test --help
Usage of ./file.test:
  -x string
    	sets a value for x (default "y")

versus go1.12, which display the entire list of flags.

The issue is present because at the time the main package parses the flags, test flags are not yet attached.
@gbbr gbbr added GoCommand cmd/go and removed GoCommand cmd/go labels Sep 21, 2019
@ALTree
Copy link
Member

ALTree commented Sep 21, 2019

Please search the issue tracker before opening a new one

#31859
#33475
#33774
#33869
#34073
#34078
#34155

Or read the release notes:

Testing flags are now registered in the new Init function, which is invoked by the generated main function for the test. As a result, testing flags are now only registered when running a test binary, and packages that call flag.Parse during package initialization may cause tests to fail.

https://tip.golang.org/doc/go1.13#testing

@ALTree ALTree closed this as completed Sep 21, 2019
@gbbr
Copy link
Member Author

gbbr commented Sep 21, 2019

Sorry, I missed the release note, and I did search the tracker 😞

@gbbr
Copy link
Member Author

gbbr commented Sep 21, 2019

Never the less, this is a terrible regression

@tejal29 tejal29 mentioned this issue Oct 4, 2019
Closed
@anphy
Copy link

anphy commented Oct 24, 2019

terrible

@cespare
Copy link
Contributor

cespare commented Oct 24, 2019

@gbbr the reason we considered this an acceptable change is that calling flags.Parse in an init was always arguably incorrect. There might be other packages that have not yet registered their flags at that point.

I suggest you modify your code to avoid calling flags.Parse inside init.

@gbbr
Copy link
Member Author

gbbr commented Oct 25, 2019

@cespare I definitely don't disagree with that. I guess I should've been more specific to say that what I meant by "terrible" was the fact that the output given by the go test command does not make the problem obvious at all and instead throws an apparently unrelated message, but I guess that's being addressed in another issue IIRC.

@mszostok mszostok mentioned this issue Nov 7, 2019
Merged
10 tasks
@arthfl arthfl mentioned this issue Jan 16, 2020
Merged
3 tasks
@golang golang locked and limited conversation to collaborators Oct 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@cespare @ALTree @gbbr @gopherbot @anphy