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.

Sign up for GitHub

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

Jump to bottom

cmd/internal/moddeps: confusing error message when difference is due to a "replace" directive #44508

Open
bcmills opened this issue Feb 22, 2021 · 3 comments
Open

cmd/internal/moddeps: confusing error message when difference is due to a "replace" directive #44508

bcmills opened this issue Feb 22, 2021 · 3 comments
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Backlog

Comments

@bcmills
Copy link
Contributor

bcmills commented Feb 22, 2021

On CL 295150 Patchset 2, I suspected that the TryBots would fail due to the replace directive, but I wanted to see what would happen anyway.

TestDependencyVersionsConsistent did indeed fail as expected, but the last error in the output is very weird:

--- FAIL: TestDependencyVersionsConsistent (0.00s)
    moddeps_test.go:361: cannot check consistency for filesystem-local replacement in module cmd (/tmp/workdir/go/src/cmd):
        # golang.org/x/mod v0.4.1 => /usr/local/google/home/bcmills/x/mod
    moddeps_test.go:379: Modules within GOROOT require different versions of golang.org/x/mod.
    moddeps_test.go:391: cmd	requires v0.4.1 => /usr/local/google/home/bcmills/x/mod
    moddeps_test.go:391: cmd	requires =>

The correct reason for the failure is the one reported at line 361 (cannot check consistency for filesystem-local replacement).

However, the failure also includes an extraneous line cmd requires =>, which is quite confusing.
@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure. help wanted labels Feb 22, 2021
@bcmills bcmills added this to the Backlog milestone Feb 22, 2021
@bcmills
Copy link
Contributor Author

bcmills commented Feb 22, 2021

CC @dmitshur

@dmitshur
Copy link
Contributor

Can this be fixed by changing t.Errorf("cannot check consistency for filesystem-local replacement in module ... into t.Fatalf? It seems the rest of the test doesn't produce a helpful output if there are filesystem replacements. Or is there more to do here?

@bcmills
Copy link
Contributor Author

bcmills commented Feb 22, 2021

t.Fatalf might be acceptable, although ideally we should report all of the differences in all of the modules, rather than one diff in a single module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
2 participants
@dmitshur @bcmills