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

[go] tests/GoTest.sh: Fix flags.Parse location to work on new go SDKs. #6388

Merged
merged 2 commits into from
Jan 7, 2021

Conversation

reltuk
Copy link
Contributor

@reltuk reltuk commented Jan 7, 2021

Calling flags.Parse() within init() races with other packages which register
flags in their init(), and in particular with the testing package itself. It is
more reliable to call flags.Parse() from a TestMain implementation.

See golang/go#31859,
golang/go#33869.

Calling flags.Parse() within init() races with other packages which register
flags in their init(), and in particular with the testing package itself. It is
more reliable to call flags.Parse() from a TestMain implementation.

See golang/go#31859,
golang/go#33869.
@github-actions github-actions bot added the golang label Jan 7, 2021
@reltuk reltuk changed the title tests/GoTest.sh: Fix flags.Parse location to work on new go SDKs. [go] tests/GoTest.sh: Fix flags.Parse location to work on new go SDKs. Jan 7, 2021
@reltuk
Copy link
Contributor Author

reltuk commented Jan 7, 2021

Before this change, I get this when trying to run the tests:

aaronson@Aarons-MacBook-Pro tests % go version
go version go1.15.5 darwin/amd64
aaronson@Aarons-MacBook-Pro tests % ./GoTest.sh
flag provided but not defined: -test.timeout
Usage of /var/folders/zj/r1b6gjbx1vn5rxvyf8tzsldm0000gn/T/go-build819444557/b001/flatbuffers_test.test:
  -cpp_data string
    	location of monsterdata_test.mon to verify against (required)
  -fuzz
    	perform fuzzing
  -fuzz_fields int
    	fields per fuzzer object (default 4)
  -fuzz_objects int
    	number of fuzzer objects (higher is slower and more thorough (default 10000)
  -java_data string
    	location of monsterdata_java_wire.mon to verify against (optional)
  -out_data string
    	location to write generated Go data
  -quickchecks int
    	The default number of iterations for each check (default 100)
exit status 2
FAIL	flatbuffers_test	0.069s
FAIL

@@ -47,11 +47,11 @@ cp -a ./go_test.go ./go_gen/src/flatbuffers_test/
# flag -test.bench and the wildcard regexp ".":
# go -test -test.bench=. ...
GOPATH=${go_path} go test flatbuffers_test \
--test.coverpkg=github.com/google/flatbuffers/go \
--coverpkg=github.com/google/flatbuffers/go \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was necessary for me...test.coverpkg was reported as not registered. I'm not sure if go test itself interprets the flag as part of compilation and doesn't actually pass it along to the test binary or something.

--bench and --benchtime below were changed for consistency, but I'm also happy to change them back. It works either way for me.

@aardappel
Copy link
Collaborator

Thanks! can you enable the build-go test here: https://github.com/google/flatbuffers/blob/master/.github/workflows/build.yml as that likely should now also work.

@aardappel
Copy link
Collaborator

@rw

@github-actions github-actions bot added the CI Continuous Integration label Jan 7, 2021
@aardappel
Copy link
Collaborator

Yay, working Go CI, thanks! :)

@aardappel aardappel merged commit 41253e5 into google:master Jan 7, 2021
@reltuk
Copy link
Contributor Author

reltuk commented Jan 7, 2021

That's awesome. Thanks for quick turn around!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration golang
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants