-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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: support compiling all tests without running #15513
Comments
Is |
And I want this, but I do want the test binary. I'd use it to compile tests, change the compiler, compile tests again, and then run before/after benchmarks. (All in a script, of course.) Like toolstash for std tests. I currently do something similar but with It seems to me we could generate a testmain that invokes all the tests in all the included packages. There are some questions: How do you know from the -v output which package this particular instance of TestReader is from? How does the -run filter work? And so on. My hand-waving answer is: Mimic the UI for subtests and sub benchmarks, which already have this nesting problem; the nesting is just up a level rather than down. |
we can't generate one binary for tests in multiple packages
because there is no guarantee that package A's test doesn't
interfere with package B's use of package A.
|
What if we leave that up to the user to diagnose? If package A's test interferes with package B's use of package A, then don't test them together. This sort of issue can already arise in a single package. |
how could the user know that one can't test package A and package B
together?
consider "go test -c ./...", which could easily contain hundreds of
packages.
|
They'll know when the test fails. If the tests pass when run independently (i.e. |
Even if they could know the bad interaction between A & B (it's
not that easy because given that B's tests failed, you still don't
know which of B's dependent [both directly and indirectly] might
be the cause. What's more, the behavior also depends on the
order tests for different packages are run.) because cmd/go
doesn't support negative patterns, there is no way to use ./...
and exclude B.
Given a large enough GOPATH, it's very likely there are bad
interactions between the package tests, and then basically
the new feature is useless (in the worse case, the user has
to name all the packages on the command line, and it's no
better than status quo.)
We might be able to do this if multiple packages are compiled
into a single package from the beginning, but I don't think we
can add this feature now.
|
I can't imagine someone wanting to run
Yes, this is unfortunate. I apologize, but I don't quite understand the resistance here. It seems like the resistance is "something could go wrong". But this doesn't really seem like a big footgun to me. And when this is helpful, like cross-compiling all tests for a big project or keeping around an old version for benchmarking or testing compilation speed, it is very helpful, and users will be motivated to make it work (or not use it). And it is entirely backwards compatible. |
I think this issue gives one reason for do go test -c ./... for the The key point is most tests are not prepared for this. And how do you propose to handle conflicting flags added Also note that tests for a package needs to run at the (FTR, I have been thinking about this feature a long time |
Exactly. And they got fixed because people cared. Same situation here, I'd say.
Sorry, what do you mean by this?
Refuse to compile, with a lucid error message. Rare.
os.Chdir? Note that the go command has the directory available at compilation time. This is a bit ugly for the cross-compilation case, but we could print a warning instead of failing when os.Chdir fails.
Yeah, I recall there being another issue about this but couldn't find it. I agree that there are limitations, I just see them as limitations rather than showstoppers. |
By conflicting flags added by tests, consider two The code will compile, but will fail at runtime with There are just too many global states, hence I |
I'm new to Go, but FWIW, the lack of this feature is one of the most frustrating things that I have found so far about the tooling. I tend to write production code and test code at the same time, and during my normal dev cycle I want to see if things compile many more times than I want to run the tests. So right now I'm doing roughly the same thing as stated above inside my project with a few packages: basically for i in It would be great if this could be done in one pass with a single link and have it be up to the user to deal with conflicts as suggested by @bradfitz |
If you want to compile but run not tests,
Replace nope with another pattern if you have test cases that match. On Fri, 27 May 2016, 03:45 Matt Klein notifications@github.com wrote:
|
@davecheney I'm probably doing something stupid but running that command still causes all the tests to execute. I tried: go test -run nope ./... Also still runs tests. |
Please try -run=nope On Fri, 27 May 2016, 09:56 Matt Klein notifications@github.com wrote:
|
I tried both:
|
-run=nope and -run nope are equivalent. @mattklein123 no tests are being run (probably). Use -v to confirm. Please use the mailing list if you have further questions about go test. |
That is correct. The test binary is built and executed, but none of the On Fri, May 27, 2016 at 10:11 AM, Caleb Spare notifications@github.com
|
i prefer -run ^$ |
My motivation is for the builders, and test sharding. The problem with |
@bradfitz, I think for your purpose, having multiple go test -c processes
is better than figuring out where to store the compiled binaries when
building multiple tests (the only safe way is to put the compiled test
binary in their own package directories, but then you need to collect them.
It's probably easier to just go test -c with explicitly output filename).
It won't do much more work either (assuming go install std cmd has passed.)
|
I can do it myself, but my concern is that I can't keep the CPU busy as well as the cmd/go binary could. cmd/go can load the dependency graph once, come up with a plan, and keep a certain number of worker processes running at all times. Any coordination I do trying to run multiple cmd/go binaries wouldn't be as good. Definitely low priority, though. |
You can speed things up a lot faster with https://gist.github.com/howardjohn/c0f5d0bc293ef7d7fada533a2c9ffaf4 by avoiding linking the binaries. A warm run on even huge repos like k8s can do this in 1-2s on my machine. |
@howardjohn, it may be faster but it's not the same. Link-time errors are possible in pure Go programs and even fairly common in cgo programs. |
Change https://go.dev/cl/466397 mentions this issue: |
At last! Excited to see this supported by the toolchain, will avoid nasty hacks like this in the future 😂 |
So, one minor detail: for a single package, For consistency, should |
How would conflicts be avoided here, with multiple packages of the same name but different paths? Just thinking about a general solution for different use-cases... If |
Conflicts would cause the
I think that's probably overkill. I expect that the vast majority of uses will be |
Thanks for the explanation @bcmills
Just saw the test for this in the CL, apologies I missed it the first time. However, being able to robustly build all test binaries would seem necessary to satisfy e.g. @bradfitz's comment about sharding them over the network (it's an old comment but I can still see reasons we'd want to be able to do this). For my own use cases, I've written scripts using |
Yeah, I agree that those cases exist but I think the solution for them is, as you mentioned, to have those cases run |
Change https://go.dev/cl/488275 mentions this issue: |
Currently, "dist test -compile-only" still runs the test binaries, just with -run=^$ so no tests are run. It does this because, until recently, "go test -c" would fail if passed multiple test packages. But this has some unexpected consequences: init code still runs, TestMain still runs, and we generally can't test cross-compiling of tests. Now that #15513 is fixed, we can pass multiple packages to "go test -c". Hence, this CL make dist just use "go test -c" as one would expect. Found in the course of working on #37486, though it doesn't really affect that. Change-Id: If7d3c72c9e0f74d4ea0dd422411e5ee93b314be4 Reviewed-on: https://go-review.googlesource.com/c/go/+/488275 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Austin Clements <austin@google.com>
Change https://go.dev/cl/500956 mentions this issue: |
Change https://go.dev/cl/500955 mentions this issue: |
Also handle go test -c TODO. For #15513. For #56986. For #57001. Change-Id: I571ae25d8d8fcd44cb38ac16cdd2a1180016eb94 Reviewed-on: https://go-review.googlesource.com/c/go/+/500956 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: David Chase <drchase@google.com>
This was missing from CL 466397. For #15513. Change-Id: I138b7d76842815f4e702d7fe551aa8968097f75b Reviewed-on: https://go-review.googlesource.com/c/go/+/500955 Reviewed-by: Michael Matloob <matloob@golang.org> TryBot-Bypass: Russ Cox <rsc@golang.org>
This is super useful, thanks everyone involved. I opened a followup issue here: #61199 that I think can help make this more useful. |
* [goreleaser] * up to calling submitStateReport * barebones report invalid state in snapshot. need to update root PR * make contract interfaces return txs for submitter * [goreleaser] * Cleanup: setup in parallel * staging: working on fraud_test notary signature * WIP: working thru fraud test * WIP: working test * Cleanup: remove logs * Cleanup: more logs * Fix: throw error for int64 overflow in EncodeState() * WIP: fraud attestation handling * WIP: add TestReportAttestationNotOnSummit * WIP: testing. not getting log for submitAttestation event * WIP, getting error of datahash code 0x184fb2df0de4da8049867b20a0d8b823d4590d0f3c9881144e53690eb2f67fe7 * WIP: successfully submitting fraudulent attestation. now debugging guards report * WIP: working TestReportAttestationNotOnSummit * Cleanup: test, remove logs * Cleanup: remove GetAttestationComputeHash(), instead calculate hash as input * Cleanup: remove unnecessary TestEncodeChainGasParity for now * Cleanup: remove init() used for debugging lightinbox.metadata.go * Cleanup: de-functionalize slashAccusedAgent() for consistency * WIP: adding submitAttestationReport * working submitattestationreport * WIP: getting a signature for the STATE report * small changes before handoff * WIP: verifyStateWithAttestation case impl * Feat: add TestReportFraudulentStateInAttestation * staging. added a getter to the bondingmanager * working test for fraudulentstateinattestation * WIP: add receipt type with encoder * WIP: add TestInvalidReceipt and handleReceipt impl * CLEANME: added checks to test and start with tips encoding parity * WIP: working case 1 of agnetStatusUpdate * WIP: opendispute handling * Cleanup: guard impl * Cleanup: remove checks from event parser methods * start of db implementation * fix * fillbytes * fix * staging * Cleanup: remove printing reverts * WIP: add getTestGuard() helper * working receipt fraud test * Cleanup: add bumpTx helper * Cleanup: utilize AgentFlagType enum for AgentStatus.Flag values * Cleanup: use getTestGuard() helper in all tests * Cleanup: remove unnecessary test code * Feat: add DisputeStatus type, GetDisputeStatus() call to BondingManager * Feat: guard checks dispute status before submitting state report * Cleanup: remove embedded lightinbox * Cleanup: Domain -> AgentDomain * WIP: verify agent status before slashing * working multiple state case * start to sql queries for crosstable dispute handling * WIP: implement db queries and logic for updateAgentStatus() call * WIP: fix cyclic deps * Fix: build errs * Add db tests * wip: crosstable * passing TestGetUpdateAgentStatusParameters * working crosstable testrs * WIP: add updateAgentStatuses() * Feat: working anvil backend for fraud tests * WIP: add evm.IncreaseTime() call * clean and reformat query * WIP: add bumpBackends() * WIP: merge handling of DisputeOpened on remote / summit * Cleanup: remove logs * Fix: build * Cleanup: some lints; guard parses RootUpdated events * Cleanup: more lints * Cleanup: remove unnecessary param in VerifyStateWithSnapshot() * Cleanup: more lints * Cleanup: more linting * Cleanup: remove comments * Cleanup: merge isStateSlashable() logic * Cleanup: unused params * Cleanup: another unused param * Cleanup: move event parsers to utils.go * Cleanup: move handlers to fraud.go * Cleanup: add comments for fraud handlers * Cleanup: comments * De-duplicate agent signing logic (#1228) * WIP: merge common signing logic into sign() util * WIP: add Encoder interface for signed types * Feat: add consts for salt values * Cleanup: remove specific language from generic signEncoder func * Fix: parity tests * Cleanup: lint * Fix: don't parse notary err on ctx cancel * Cleanup: enable TestGuardE2E * Cleanup: err msg * Fix: ignore ctx cancel err in agents integration test * Fix: TestAgentsE2E * Fix: avoid int64 overflow in executor tests * Cleanup: resolve merge conflict * Cleanup: merge conflict * Feat: add NowFunc for time override * WIP: executing mngr msg * WIP: debugging GetLatestSummitBlockNumber() * wip: friday progress * Feat: add PassAgentRoot to destination interface * WIP: parse RootUpdated on light manager, submit extra attestation and bump optimistic period on destination * WIP: manually pass in agent root in last attestation * Feat: guard db uses GetBlockNumberForRoot() * WIP: working test * Feat: correct block number condition * Cleanup: remove logs * Cleanup: more logs * Cleanup: more cleanup * Cleanup: remove unnecessary event parsing, re enable guard submitter * add GetSummitBlockNumberForRoot test * Cleanup: logs * Fix: re enable submitter in notary * Cleanup: go mod tidy * small lint * reduce again * support for remote scribe in guard commands * trivial fixes * Fix: retry contract deployment * Extra state reports (#1287) * Feat: submit state reports to remote chains * Feat: add SubmitStateReportWithAttestation on LightInbox * WIP: verify number of reports on agent domain * Feat: add prepareStateReport() for all report submissions * Feat: add working state report verification to TestReportFraudulentStateInAttestation * Feat: add verifyStateReport() helper * Feat: add verifyStateReport() helper * WIP: add RelayableAgentStatus model * Cleanup: remove unused dispute handling * WIP: working TestFraudulentStateInSnapshot with new agent status model * WIP: fix crosstable logic, logs * Feat: add NotaryOnDestination to test setup * WIP: working test with NotaryOnDestination and Order clause in db query * Cleanup: logs, dead code * Fix: db tests * Cleanup: lints * Cleanup: remove Debug() gorm flags * Feat: add updateAgentStatus() test helper * Cleanup: update db comment * Cleanup: correct db comment * Fix: some lints * Cleanup: lint * Cleanup: clear nonce * Submitter and Retries for Fraud (#1288) * wrap each func. need to fix test * working tests with submitter * remove test file * start to sql queries for crosstable dispute handling * retries and print statements to be deleted * undo generic retry * remove test files * remove other test file * working tests * remove print statement in executor * implement wasse suggestions * [goreleaser] * fraud report branch * gosum * upterm * assert->require * log json receipt of failed txes * receipt * try nightly (this is a longshot) * shorten ci runs * shorten ci runs * throw reverts at deployer level in addition to solidity level * throw reverts at deployer level in addition to solidity level * test some ideas * remove for * hi * redundant * generation fix [ci skip] * fatal * summit * real chain id * update * uptemr * [ci skip] * fix host port to not be bound to hostname * Fix: add index to avoid 'ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint' error * upterm * REVERT THIS. test changing all jaeger to null * re enable workflow * Revert "re enable workflow" This reverts commit f5ca330. * Revert "REVERT THIS. test changing all jaeger to null" This reverts commit 2320a1c. * start to sql queries for crosstable dispute handling * dont run upterm * upterm * Fix: log tx hash within submitter * Fix: do MIN(block_number) in db query * error instead of warn * Delete agents/agents/guard/test.txt * Delete agents/agents/test.txt * no jaeger in CI * upterm * Add logs * add agent status prints * omnirpc and increase evm increaase time by 30 secs * fix: DDDDDDDD * no more jaeger in ci * Retry guard constructor * Disable upterm * Add check for nil guard * Enable upterm * Enable test without pyroscope / cov * Bump timeouts * Add back env vars * Add upterm * Log err on anvil purge * Lint * Add time log * Printf * Add address to anvil log * Remove mem limit and enable gc * max retry to 60 secs * maxTotalTime on guard construcotR * swing for the fences * agents lint * core lint fix * Fix: yaml lint * Revert "Add address to anvil log" This reverts commit 3e401d7. * Add codecov back to workflow * try with run instead of command * Use command instead of run with multiline content * clean comments * Fix: yaml lint * redo go.yml * Cleanup: gh action retries * Cleanup: remove getTestGuard() retry * working with some noncemanager removals * working with all noncemanager removals * precompile tests * up the memlimit [no_skip] [goreleaser] * hi [goreleaser] * fix malformed memlimit * reintroduce pyroscope [goreleaser] * try to fix codecov * github * rename coverage.txt to profile.cov * utilize precompilation (see: golang/go#15513) * f * try disabing pyroscope again * list containers at the end [revert me] [goreleaser] * fix the issue * minimal log collection * go.yml * tmp logs * Fix: use iota+1 for contract type * fix: anvil test and delete debugging * Fix: address flakes in TestUpdateAgentStatusOnRemote by bumping backends * Cleanup: add grouped stdlib imports in editorconfig * Cleanup: add TODO * fix: mkdir for tmp logs for tar logs * Cleanup: use dockerutil.GetPort() * Cleanup: isNotSummit -> isNotary * Cleanup: more descriptive comment on GetPort() behavior * create parent dir for tmp logs * fix: add iota + 1 where applicable * Fix: bump backends after updateAgentStatus() * use different dir for getting the logs * revert iota + 1 in agents/contract * undo iota + 1 in agents/types * Add logs * sleep pass * Revert "Add logs" This reverts commit 8637cb6. * Fix: verify snapshot states before bumping evm time * Cleanup: lint * Config: bump retries * Config: add on_retry_command to rm docker containers --------- Co-authored-by: Max Planck <maxplanck.crypto@gmail.com> Co-authored-by: Trajan0x <trajan0x@users.noreply.github.com> Co-authored-by: Max Planck <99688618+CryptoMaxPlanck@users.noreply.github.com>
For posterity, and anyone who wants to include this in their build, the following command finds all the packages in the current directory: go list -f '{{if .TestGoFiles}}{{.ImportPath}}{{end}}' ./... So checking if all the tests compile can be accomplished like this: go test -c -o /dev/null $(go list -f '{{if .TestGoFiles}}{{.ImportPath}}{{end}}' ./...) |
I want this to work:
But I don't actually care about the binaries. I just want to test that all the tests can compile, and how fast. @randall77 also wants this for SSA coverage reasons.
The text was updated successfully, but these errors were encountered: