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

fix go-fuzz failing at Go tip, plus get testing passing again #323

Merged
merged 23 commits into from
Jun 2, 2021

Conversation

thepudds
Copy link
Collaborator

This PR hopefully addresses a couple of go-fuzz issues, while also updating tests to handle changes in cmd/go. Travis has been failing on master for go-fuzz for ~5 different reasons for several months, but with these changes, travis is now passing again on my fork.

Fixes #313
Fixes #322

golang/go#44487 was likely causing tip to fail with a panic for the go-fuzz travis tests, perhaps due to the `// +build` in slides/crash.go and elsewhere.

Also update the tested versions of Go.
Also, temporarily use thepudds/go-fuzz-corpus fork.
…etting 'go 1.17' directive

Workaround dvyukov#313.

cmd/go should in theory pay attention to the version set in the 'go' directive when interpreting //go:build comments,
Avoid preserving '//go:build' comments in both trimComments and initialComments.

Fixes dvyukov#313.
@thepudds
Copy link
Collaborator Author

Currently running travis tests:

https://travis-ci.org/github/dvyukov/go-fuzz/builds/773061221

.travis.yml Outdated
@@ -65,7 +65,7 @@ script:
- testscript -v testscripts/mod_vendor.txt

# Prepare to test the png example from dvyukov/go-fuzz-corpus.
- go get -v -d github.com/dvyukov/go-fuzz-corpus/png
- git clone --depth=1 https://github.com/thepudds/go-fuzz-corpus $GOPATH/src/github.com/dvyukov/go-fuzz-corpus
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we clone the different repo (thepudds) into go-fuzz-corpus?
Do we need some fix in go-fuzz-corpus as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Dmitry 👋 thanks for the review!

Do we need some fix in go-fuzz-corpus as well?

The go-fuzz README likely needs to be updated regarding how to use go-fuzz-corpus (#317), and/or go-fuzz-corpus could become one or more modules, including to avoid #317 (comment). That was also the root cause of one of the ~5 reasons this testing was failing. I was taking an initial stab at a PR for go-fuzz-corpus, as well as using a go version directive in a go-fuzz-corpus go.mod as a potential workaround for the double '//go:build' issue (#313).

That said, I then switched gears to working around the go-fuzz-corpus module issues within travis (by creating a local test module in the travis script), including to keep this PR more self-contained.

In any event, I should switch the travis git clone back to dvyukov/go-fuzz-corpus, and that was my intent for this PR. Sorry for missing that.

@dvyukov dvyukov merged commit b1f3d6f into dvyukov:master Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants