-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Improve ProgramTest errors and path handling for Go programs #11611
Conversation
Changelog[uncommitted] (2022-12-13) |
3ae90fd
to
79735a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to add directions for why the specific path was checked and how to change which path will be used.
pkgModPath := filepath.Join(dir, "go.mod") | ||
pkgModData, err := os.ReadFile(pkgModPath) | ||
if err != nil { | ||
return "", fmt.Errorf("error reading go.mod at %s: %w", dir, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little more detail in the error message directing the user to be able to resolve the issue would be helpful. E.g. something along the lines of:
return "", fmt.Errorf("error reading go.mod at %s: %w", dir, err) | |
return "", fmt.Errorf("error reading go.mod at %s. This can be set explicitly by specifying the path to the replacement, otherwise the environment variable PULUMI_GO_DEP_ROOT or GOPATH will be used: %w", dir, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to a parent function to handle both of the errors in this method
79735a1
to
f79621a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent thank you!
f79621a
to
2594058
Compare
pkg/testing/integration/program.go
Outdated
for _, dep := range pt.opts.Dependencies { | ||
// when we load a package from a relative dir string, we read the package name from go.mod | ||
// Use a literal replacement path. | ||
/* pkg is a directory path, create a replace edit string from it */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Why the different comment style?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleted, this comment doesn't make sense after the refactoring.
bors merge |
2594058
to
6642fb5
Compare
🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
Build succeeded: |
11647: Fix test failure on Windows r=AaronFriel a=AaronFriel PR #11611 assumed a unix-y error message in one of the failure cases of the test `TestGoModEdits`. This case did not match on Windows: ``` === FAIL: testing/integration TestGoModEdits/invalid-path-non-existent (0.00s) program_test.go:170: Error Trace: C:\a\pulumi\pulumi\pkg\testing\integration\program_test.go:170 Error: Error "error reading go.mod at ../../../.tmp.non-existent-dir: open ..\\..\\..\\.tmp.non-existent-dir\\go.mod: The system cannot find the path specified." does not contain "open ../../../.tmp.non-existent-dir/go.mod: no such file or directory" Test: TestGoModEdits/invalid-path-non-existent --- FAIL: TestGoModEdits/invalid-path-non-existent (0.00s) ``` Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
This improves and expands on ProgramTest behavior for initializing Go programs, enabling better relative path handling and adding some safeguards. These are documented in
TestGoModEdits
.