-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
cmd/go: exclude standard library from "all" pattern in modules #26317
Comments
I guess probably "all" should exclude the standard library (in all commands not just |
Should |
Actually, there is currently a special postprocessing phase for Still, it seems like an interesting edge-case to consider. |
I stand corrected again. Patterns involving |
Change https://golang.org/cl/125835 mentions this issue: |
Per discussion on https://golang.org/cl/125835, I think this is working as intended: "all" should include the subset of the standard library that is actually (transitively) imported by the current module. To test all packages in the current module and its submodules, you can execute It's true that we don't have a good pattern for “exactly the packages in the current module”, but |
Thanks. What I was after was a way of testing all packages in the current module and the transitive closure of package dependencies, excluding the standard library, i.e.
Edit: updated command to include Reason being, it's probably safe to assume that the standard library packages I'm using (transitively) are already tested 😄. Build caching would obviate this concern to an extent, but I'd still have to run the tests for a standard library package once per release... but then that's already been done for me. Unless I'm missing something here about the benefit of (re)testing and hence including standard library packages in this definition?
I don't think However
Given the above, I think |
We do have kind of a weird inconsistency here, though. |
Change https://golang.org/cl/128637 mentions this issue: |
It's a pity we staying with the behaviour From https://research.swtch.com/vgo-tour:
Guys, I believe we definitely need some short pattern to test the entire project with dependencies excluding std lib. Because as @myitcv mentioned, why should we ever test it if it's been already tested in Go release? Patterns like And the second reason for excluding std lib - some tests are very slow, I don't want to run them even once. Consider this small example (I still have
46 sec? 20 sec? Guys, really? I see it and I just want to cancel :) And
PS Just tried to run it on
Maybe it's connected with #26722 and fixed on tip. Will check |
Yeap, working on tip. Actually I can't understand why the tests are so slow.
Why are the same tests so much faster when building go vs Anyway with so slow stdlib tests |
This is the full timing of
Strange thing that testing Anyway, if we don't count the
Does it worth testing them at all? |
@bcmills just to confirm my understanding (because this is the first time I'd seen the
True, but we also don't have a way to easily list (and therefore test) the subset of I don't actually find
gives: Lots of output
Whereas what I'm actually interested in is (ignoring build constraints for now):
|
The more I think about it, the more I think the current patterns are correct. Here's my reasoning:
So I think this is mostly just a documentation issue: we should be recommending |
Not just subdirectories, but excluding submodules in general (even if they aren't stored in subdirectories).
No, this one is “all packages in all modules with prefix
Yep.
Correct: |
Is this really what you want for pre-release testing though? Because given my example in #26317 (comment) this would involve me testing |
Given that we don't have any other means to specify “package If caching is working properly, the overhead of “running” those tests should generally be fairly minimal — especially if we split the |
I see what I was missing, thanks.
True, but an interesting/worthwhile idea though.
Indeed. In addition to this point, the bit I clearly forgot in all of this is the first line of
which helps to make more sense of the definition for Thanks for taking the time to talk this through. Do you want to leave this issue open in order to address:
?? Will leave up to you. |
The standard library is incorrectly excluded from We should be encouraging people to use |
@rsc do you mean we should exclude standard library from |
all means "all your dependencies - the things that actually get built into the packages in your modules". I don't see a compelling reason to exclude fmt if you use fmt but not exclude something else 10 levels down. Especially if you use -short, it should not be painful. And it should cache very well. |
Thanks for the clarification! OK, let's try to live with it. Only time could judge wether the community accepts this behaviour or not 😄 Maybe it's reasonable for a case, when there is really something wrong in your OS, making standard library tests to fail :) And it could help us to debug some crazy cases in future go releases. |
This change replaces https://golang.org/cl/125835. Updates #26317. Change-Id: I38ae1f93e5f5c86737a4b489df498c18b179781d Reviewed-on: https://go-review.googlesource.com/128637 Reviewed-by: Russ Cox <[email protected]>
I think this is now working as documented, so closing. |
I'm on a very recent Macbook (Pro, 2018, 6 cores): Running go test std with GO111MODULE=on takes (even with everything cached):
because of
Without the tests cached:
I think there's a bad assumption here that the std tests "should" always succeed (and that we can handwave them away with caching), because they do not, and from cold-start they are very time-consuming. I'd like to do go test all as a quick check that everything I'm depending on works together, it's not convenient or a quick part of my workflow if it can blow out like this so easily. |
If you find that a test in |
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?What did you do?
What did you expect to see?
I'm not totally clear what to expect because I couldn't see exactly where the special
all
argument is documented? But my guess is that it shouldn't include standard library packages.What did you see instead?
The text was updated successfully, but these errors were encountered: