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

introduce test_globs field for go_package to identify test files #12428

Closed

Conversation

tdyas
Copy link
Contributor

@tdyas tdyas commented Jul 27, 2021

The current design for the Go plugin has go_package own all Go sources in a directory. It is still necessary to exclude test files, however, when building a go_binary.

This PR introduces the test_globs field for go_package as the way for the Go plugin to identify which files in a go_package are test files. It defaults to ("_*test.go",) which should match the go tooling. It is a field versus hard-coded to let users override if necessary.

[ci skip-rust]

[ci skip-build-wheels]

[ci skip-rust]

[ci skip-build-wheels]
@tdyas
Copy link
Contributor Author

tdyas commented Jul 27, 2021

If the need to override proves unnecessary, then this could be hard-coded at a later time. I am concerned about what to do with resource files versus Go sources files that may or may not to be in a test package. For now, this is fine, and still allows me some measure of control during prototyping without introducing a go_tests target.

@benjyw
Copy link
Contributor

benjyw commented Jul 27, 2021

The current design for the Go plugin has go_package own all Go sources in a directory. It is still necessary to exclude test files, however, when building a go_binary.

Can you clarify why this is necessary? Doesn't the go tooling take care of this already?

@tdyas
Copy link
Contributor Author

tdyas commented Jul 27, 2021

Seems so. I wrote this PR earlier today before I wrote #12429 and learned the package metadata lists the regular and test files. We should probably just rely on that analysis by the go tooling.

I'll make this PR just remove the test file exclusion and just have GoSources owns all Go files in the directory.

@benjyw
Copy link
Contributor

benjyw commented Jul 27, 2021

Makes sense. The more we work idiomatically with the go command the better. Since it knows to exclude test files from binaries, we should just rely on that :)

@benjyw
Copy link
Contributor

benjyw commented Jul 27, 2021

It seems like the test files can be said to belong to the package, in go idiom, so...

@tdyas
Copy link
Contributor Author

tdyas commented Jul 27, 2021

Closing in favor of just taking care of this in #12429.

@tdyas tdyas closed this Jul 27, 2021
@tdyas tdyas deleted the golang_test_sources_in_go_package branch July 27, 2021 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants