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

cmd/go: go test -fuzz='^FuzzName$' uses default value of -run flag and runs all regular tests as well #50432

Open
dnwe opened this issue Jan 4, 2022 · 6 comments
Labels
fuzz Issues related to native fuzzing support GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dnwe
Copy link

dnwe commented Jan 4, 2022

What version of Go are you using (gotip version)?

$ gotip version
go version devel go1.18-95b240b2 Mon Jan 3 23:45:12 2022 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes, this was reproduced on latest tip (95b240b) and had also been seen to reproduce on my previously installed version of gotip from Sep 22nd (09d3df0) too

What operating system and processor architecture are you using (go env)?

go env Output
darwin/amd64

What did you do?

  • wrote a simple fuzz_test.go and attempted to run it with gotip test -fuzz=FuzzName as per the docs, but was surprised to find it taking a long time to run
  • added -v to see the output and noted that all of my others tests were being run too

What did you expect to see?

I expected -fuzz=^FuzzName$ to match the docs and spec and behave like -run=^TestName$:

In order to run a fuzz test with the mutation engine, -fuzz will take a regexp which must match only one fuzz test. In this situtation, only the fuzz test will run (ignoring all other tests)

What did you see instead?

gotip test proceeded to execute all of my regular tests in addition to the single fuzzing test that I'd specified. A workaround was to change the cmd to gotip test -fuzz='^FuzzName$' -run='^FuzzName$' and then it behaved as expected, running a single fuzzing test and nothing else.

@thepudds thepudds added the fuzz Issues related to native fuzzing support label Jan 4, 2022
@mvdan
Copy link
Member

mvdan commented Jan 4, 2022

Interesting to see that we have a test which specifically locks this behavior in:

# Fuzz successful chatty fuzz target that includes a separate unit test.
go test -v chatty_with_test_fuzz_test.go -fuzz=Fuzz -fuzztime=1x
stdout ok
stdout PASS
! stdout FAIL
stdout -count=1 'all good here'
# Verify that the unit test is only run once.
stdout -count=1 'logged foo'

It seems like it was added in b894834 by @katiehockman. It sounds like a relatively straightforward fix, be it amending the docs, or updating the code to follow the docs. The simplest fix in terms of code might be to have -run default to the value of -fuzz if -fuzz is set but -run is not.

@katiehockman
Copy link
Contributor

@dnwe thanks for filing this. Can you clarify where you saw these docs. If it is only in the design draft, that doc is out of date (see the addendum at the top of the doc), so that doesn't need to be updated.

This is intentional behavior. See #46222

@katiehockman katiehockman added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 4, 2022
@katiehockman katiehockman added this to the Backlog milestone Jan 4, 2022
@dnwe
Copy link
Author

dnwe commented Jan 4, 2022

@katiehockman in terms of docs, it probably was only the draft proposal that made me think the behaviour wasn’t matching the design, or perhaps something I saw in the old go help fuzz output. After re-reading gotip help testflag I can see the current behaviour is specified there:

-fuzz regexp
	    Run the fuzz test matching the regular expression. When specified,
	    the command line argument must match exactly one package within the
	    main module, and regexp must match exactly one fuzz test within
	    that package. Fuzzing will occur after tests, benchmarks, seed corpora
	    of other fuzz tests, and examples have completed. See the Fuzzing
	    section of the testing package documentation for details.

but it would be great if we could cover it on https://go.dev/doc/fuzz/ too and also highlight the difference between gotip test runs that only use the seed corpus, and -fuzz=FuzzXYZ runs that run through mutations to generate fuzz testdata

EDIT: I've also now read through the latest https://pkg.go.dev/testing@master#hdr-Fuzzing which probably should have been my starting point, rather than https://go.dev/doc/fuzz/ as it does explain and even has "The -fuzz and -run flags can both be set, in order to fuzz a target but skip the execution of all other tests"

I guess one of the unexpected parts of the current behaviour is that even if I've already run through all of the tests myself via gotip test ., the testcache gets invalidated when I then subsequently run gotip test -fuzz=FuzzTest . straight after, so then it will start by re-running all of the tests again even though I've just run them and haven't changed any files. So if I'm wanting to perform some fuzztime for all of my fuzz tests (gotip test -list . | grep '^Fuzz' | parallel -j1 'gotip test -v -run=^{}$ -fuzz=^{}$ -fuzztime=10m') the regular tests, examples, benchmarks would get re-run for each one.

It's a shame we can't preserve the testcache ignoring -fuzz= parameters.

@katiehockman
Copy link
Contributor

That's an interesting idea. I can see your point about that fact that running go test with -fuzz could potentially use some of the cached test data from previous runs of go test. @bcmills do you have opinions on the feasibility of this? I would need to think about it some more.

One possible issue is that the binary isn't exactly the same, because running go test -fuzz instruments the testing binary in a way that's different than typical runs of go test. That instrumentation should be irrelevant for the unit tests, but it's there nonetheless.

/cc @golang/fuzzing

@katiehockman katiehockman added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jan 4, 2022
@disconnect3d
Copy link

disconnect3d commented Mar 28, 2023

Bump. Can we get this poor UX fixed? It is annoying and unexpected that passing in -fuzz still runs normal tests (and you have to repeat the harness name like go test -v -fuzz='^FuzzMyFunc$' -run='^FuzzMyFunc$'), not to mention the fact that go test without -v does not list all the tests that it executes (it will execute tests, but the output will only show failures), but oh well.

@disconnect3d
Copy link

disconnect3d commented Mar 28, 2023

I would also like to offer some other suggestions regarding UX of fuzzing:

  1. It may be good to move go test -fuzz to just go fuzz to distinct between the two better and avoid mistakes of running tests vs fuzzing, but it's not really a big deal
  2. There should be a command to list all harnesses in all packages recursively and only that. Now, it seems we can list tests via go test -list . in current package. But it has two big downsides:
  • First, the last line it prints will be something like "ok 0.023s" - we do not need that!
  • Second, I would expect go test -list ./... to list tests recursively, by analogy to go test ./... which executes tests in subdirectories. However, -list will just tell "no go files in <path>". Why?
  1. It would be great to inform the user where are the corpus files saved when they finish a run, a simple message of All corpus files can be found in /home/user/.cache/go-build/fuzz/<...>. I mean, we should make the user aware this exists and that they can, e.g., store it to resume fuzzing with it or to send it to someone.
  2. We should consider keeping corpus and crashes together. I mean, that would be more useful to have a single directory with everything related to a given harness instead of splitting it around to different paths ($GOCACHE/... for corpus and testdata/... for crashes).
  3. With 2) there should be some command to re-run all harnesses in the project with -cover and to generate coverage reports. There should be: reports for each harness and then a single report with merged coverage.

I understand that this issue may not be the best place to discuss any of those. Please point me/move this post elsewhere if there is a better place to discuss them. Also, sorry for not doing an exhaustive research if those were already discussed or planned.

@dmitshur dmitshur added the GoCommand cmd/go label Sep 3, 2023
@dmitshur dmitshur changed the title testing: gotip test -fuzz='^FuzzName$' appears to run regular tests as well cmd/go: go test -fuzz='^FuzzName$' uses default value of -run flag and runs all regular tests as well Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzz Issues related to native fuzzing support GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Status: No status
Development

No branches or pull requests

6 participants