test(generator): vet every generated package, not just the corpus - #122
Merged
Conversation
#120 compiled and lost data: two fields sharing a JSON tag, which encoding/json resolves by ignoring both. go vet names that, and the corpus test was the only place running it, so 45 other tests would have watched the same bug go by. Both e2e helpers now vet what they generate: the one that compiles a package and the one that compiles and runs a wire test against it. All 62 tests pass as they stand, so this adds no fixes, only the guard that would have caught the last one. The corpus test drops its own copy of the helper and uses the shared one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#120 was a package that compiled and silently lost data: two fields sharing a JSON tag, which
encoding/jsonresolves by ignoring both.go vetnames it, and #119 had added vet to exactly one test, so the 45 other e2e tests would have watched the same class of bug go past.Both helpers now vet what they generate:
buildGenerated, used by the tests that compile a packagerunGeneratedWireTest, used by the tests that compile one and run a wire test against itAll 62 e2e tests pass unchanged, so this adds no fixes. It adds the guard that would have caught the last one.
I checked the guard actually fires rather than assuming it: with the
allOfdeduplication from #121 disabled, the corpus test fails withThe corpus test also drops the private
buildAndVetit introduced and uses the shared helper.gofmt,golangci-lint,go vet ./..., andgo test ./...pass.