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

x/tools/go/loader: tests are too brittle #19152

Closed
bradfitz opened this issue Feb 17, 2017 · 3 comments
Closed

x/tools/go/loader: tests are too brittle #19152

bradfitz opened this issue Feb 17, 2017 · 3 comments
Labels
FrozenDueToAge Testing An issue that has been verified to require only test changes, not just a test failure. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@bradfitz
Copy link
Contributor

The x/tools/go/loader tests have a history of breaking whenever something in the standard library
changes.

Or breaking when testing Go 1.6 vs Go 1.7 vs Go 1.8 vs Go tip, etc.

I think it's time to rethink those tests to not be a maintenance burden.

(Latest example: https://go-review.googlesource.com/c/37148/)

/cc @ianlancetaylor

@bradfitz bradfitz added the Testing An issue that has been verified to require only test changes, not just a test failure. label Feb 17, 2017
@bradfitz bradfitz added this to the Go1.9 milestone Feb 17, 2017
gopherbot pushed a commit to golang/tools that referenced this issue Feb 17, 2017
Tip introduces a new internal/poll package that breaks the expected
output.

Fixes golang/go#19150
Updates golang/go#19152

Change-Id: I5ff7e8a92afe4d25feb6365933062e931c9b435f
Reviewed-on: https://go-review.googlesource.com/37148
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
@bradfitz bradfitz modified the milestones: Unreleased, Go1.9 Jun 14, 2017
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
@dmitshur
Copy link
Contributor

dmitshur commented Apr 9, 2020

Another instance is #38318.

@gopherbot
Copy link

Change https://golang.org/cl/227772 mentions this issue: go/loader: delete stdlib_test.go

gopherbot pushed a commit to golang/tools that referenced this issue Apr 13, 2020
The check was there to test that the loader worked with a cycle between the
three augmented packages compress/bzip2, io/ioutil, and regexp, because of
dependencies between each of the packages' tests and the next package.

The test in io/ioutil that had the dependency that created the cycle no longer
exists in that package (it's been moved out into the ioutil_test xtest).
Remove the check for that package.

Unfortunately this means that the cycle that was being checked for before is no
longer being checked. That could be fixed in a future change by creating three
fake packages in testdata that have this relationship.

Fixes golang/go#38318
Updates golang/go#19152

Change-Id: I8ce88102a5505d8edf8d54d2098c85a8d3cd622f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227772
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@stamblerre
Copy link
Contributor

Given that golang.org/x/tools/go/loader is now deprecated, I think it's reasonable to say that no one will probably have the time/desire to invest into fixing this issue. In that case, I think we can either continue to fix breakages like #38318 as they come up (and make sure that they get caught sooner rather than later), or decide that these tests aren't worth it and disable them. Given the discussion on https://golang.org/cl/227772, I would say that we do consider these tests worthwhile, in which case I propose we close this issue and fix breakages as they happen.

@golang golang locked and limited conversation to collaborators Apr 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Testing An issue that has been verified to require only test changes, not just a test failure. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants