Resolve black box test import path from go.mod#39
Open
myzktkyk wants to merge 1 commit into
Open
Conversation
The generated gobco_bridge_test.go must import the target package by
its real Go import path. The previous implementation scanned imports
in the target directory and copied one whose base name matched the
target package's name. That heuristic failed when:
- no '_test' file imported the target package directly
(it was only reached through a helper package), and
- another import in the same directory happened to share the same
base name as the target package (for example, an interface package
"iface/target" imported from "target").
In that case, the bridge file imported the wrong package and the
test build failed with 'undefined: target.GobcoCover'.
Compute the import path instead by combining the module path read
directly from go.mod with the relative path from the module root,
so the result no longer depends on which imports happen to appear
in neighbouring files.
The added testdata/issue33 reproduces the failure scenario described
in issue rillig#33.
Fixes rillig#33
Author
|
Hi @rillig I took @1st-vil's approach in #34 as a starting point and opened #39 against current master. A few differences from #34 that I hope address the points you raised:
I also added Would you mind taking a look when you have a moment? Happy to adjust anything. |
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.
Summary
gobco_bridge_test.goby reading the module path fromgo.modand combining it with the directory's relative path inside the module, instead of scanning imports in the target directory._testpackage may fails #33 where the previous heuristic could pick an unrelated import (e.g. aniface/targetpackage) when no_testfile imports the target package directly.findInModulefrommain.gointo a package-level function so it can be reused.testdata/issue33reproducing the original failure scenario, used by the newTest_gobcoMain__issue33.Background
This PR supersedes #34. Compared to that PR, it resolves the module path by reading
go.moddirectly instead of shelling out togo list -m, which avoids depending on the cwd and ongobeing in PATH at instrumentation time, and uses thepathpackage so import paths are correct on Windows.Test plan
go test ./...passes (including the newTest_gobcoMain__issue33)sh ./test.shequivalent:go install ./...,gobco .,gobco ./testdata/instrumenter,gobco -branch ./testdata/instrumenterall succeed./gobco_bridge_test.go:6:16: undefined: target.GobcoCover)Fixes #33
Credit to @1st-vil for the original approach in #34.