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

proposal: go test ./.../... for submodule/multimodule projects #59480

Closed
cyrusv opened this issue Apr 7, 2023 · 14 comments
Closed

proposal: go test ./.../... for submodule/multimodule projects #59480

cyrusv opened this issue Apr 7, 2023 · 14 comments

Comments

@cyrusv
Copy link

cyrusv commented Apr 7, 2023

Background

As discussed in #27957,

go test does not have a way to natively run all tests in a directory or project:

  • go test ./... does not run tests in submodules
  • go test all runs all dependencies, including core library. It is no longer documented

Since submodules are now officially recommended project structure (for example, v2 documentation), it becomes valuable to have a native way to run all the tests

  • For safety: If I add a V2, and my CI relies on go test ./..., it will give me green, though the V2 test might fail.
  • Coverage: You can't even rely on coverage assertions here, since the code does not even run

Current workarounds:

  • Using shell scripts to find submodules, and running go test ./... within each submodule, as here. The drawbacks are that a) everyone copies the same hack everywhere, and behavior could be sensitive to which shell the user/CI is using and b) managing exit status in these loops is a pain -- no easy way to get the full result of the tests (coverage, all tests, etc) and also exit with appropriate status to pass/fail.
  • Explicitly track all modules in test-running script. This is fine, but easy to forget during refactors, V2s, etc, and relies on humans knowing this undocumented feature of Go.

Proposal

A new argument: go test ./.../... that will also test submodules. This way, existing integrations are unaffected, and documentation around submodule/multimodule projects can emphasize the new feature. Stylistically, the ./.../... argument aligns with the existing simple arguments.

@cyrusv cyrusv added the Proposal label Apr 7, 2023
@gopherbot gopherbot added this to the Proposal milestone Apr 7, 2023
@cyrusv cyrusv changed the title proposal: go test for submodule/multimodule projects proposal: go test ./.../... for submodule/multimodule projects Apr 7, 2023
@seankhliao
Copy link
Member

see also #50745

@cyrusv
Copy link
Author

cyrusv commented Apr 7, 2023

see also #50745

Thank you for the reference, there is a lot of useful discussion there. It sounds like a similar problem, I can continue discussion there. The title of that thread, however, frames this as a gap in Go workspaces, but I think this is a more general gap in any Go tooling that supports the ./... syntax.

@cyrusv
Copy link
Author

cyrusv commented Apr 18, 2023

@bitfield, curious what your feedback is?

@bitfield
Copy link

bitfield commented Apr 18, 2023

A module is the unit of versioning, so it makes sense to me that it's also the unit of testing. There's no need to have a single go test command test more than one module at once. Modules are, more or less by definition, self-contained when it comes to testing.

You're really asking for a change that would make it less painful to maintain multiple modules in a single repo, and I don't think that's something we want to make less painful. I think it should be painful, because it's a bad idea.

@cyrusv
Copy link
Author

cyrusv commented Apr 18, 2023

@bitfield, I'm not sure I understand -- the official docs for major version bumps explicitly recommend making a new module:

The recommended strategy is to develop v2+ modules in a directory named after the major version suffix.

Am I missing something?

@bitfield
Copy link

Well, publishing a new major version of a module (that is, breaking your established public API) isn't something we should encourage either. Rich Hickey makes this point persuasively: What if we never broke anything?. And it's worth noting that the Go language itself never does this, by design. It guarantees backward compatibility, and if Go can do it, so can we.

If you must bump your major version, I think the best approach is probably to embrace the idea that you're publishing a whole new module (which you are). Publish it in a different repo under a new name; don't bother with v2, that just confuses everybody and normalises the idea that we should be making breaking changes to our public APIs, and suggests that Go has an "official" way to do this.

@ghost
Copy link

ghost commented May 10, 2023

I dont like this. I think ./... is already "magic" enough, this is just another syntax that is impossible to internet search. One of Go strength is many decisions are made with consideration to being explicit and clear, instead of being "code golfed" or clever. this change seems to fall firmly in the second category. I would say another flag would be a better option, something like go test -submodule ./... or similar.

@rsc
Copy link
Contributor

rsc commented Jul 12, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Jul 19, 2023

The consensus here seems to be that this is too special.

@cyrusv
Copy link
Author

cyrusv commented Jul 19, 2023

Thanks for the update, @rsc -- Any link to the full discussion? Can you help me understand: Is the problem too special, or the specific proposed solution too special?

Is there agreement, at least, that there is a need for a way to run all tests in all submodules from top-level in a multi-module project with a single command?

The alternative is that we are forced to copy-paste some bash loops and make targets around stackoverflow 😓

If the answer is "Don't use sub-modules", then I am happy to take that advice. But that means there is a very large discrepancy between such a recommendation a LOT of the public documentation (e.g., v2 of a module, workspaces, etc).

@rsc
Copy link
Contributor

rsc commented Jul 19, 2023

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Jul 26, 2023

Sorry for the confusion. By "here" I meant the discussion on this very issue, not some other discussion.

@bitfield pointed out that the module is the unit of versioning so it makes sense for it to be the unit of testing as well.
@1268 pointed out that there are syntactic problems too.

To elaborate on @1268's comment, right now the meaning of ... in command-line arguments is purely lexical: it is like * or ** in certain glob syntaxes, or .* in regexps. People often use ./... but that's not a special case by itself. You can do things like go test m... to test all the packages beginning with m, go test ...plate to test all the packages ending in plate, and so on. So ./.../... also already has a defined meaning, which is not much different from ./.... (This idea of using of ... as a wildcard was borrowed from Perforce, for what it's worth, because I liked how well it worked there.) Making ./.../... have a special meaning would be an actual special case instead of a general rule.

It may be worth pointing out that all does include all the packages in your workspace (and their dependencies), and wildcards work by filtering the all list, so if you are using workspaces with your multimodule project, then you can use go test all (will include dependencies outside that module) or go test yourprefix/... to limit to roughly what ./.../... would mean in this proposal. So quite possibly the answer is "use workspaces".

@rsc
Copy link
Contributor

rsc commented Jul 26, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Aug 2, 2023

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Aug 2, 2023
knadh pushed a commit to knadh/koanf that referenced this issue Mar 7, 2024
- Define go.work to include all "sub" modules.
- Run `go test -v github.com/knadh/koanf...` which runs test
  on all references in go.work doing a prefix match of the package path
  filtering the "go test all" behaviour of testing every single dep.

  Ref: golang/go#59480 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Declined
Development

No branches or pull requests

6 participants