-
Notifications
You must be signed in to change notification settings - Fork 50
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
Updating to Go 1.19 #192
Updating to Go 1.19 #192
Conversation
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.
@@ -177,6 +178,10 @@ The program must also be executable according to the platform where Terraform is | |||
workingDir := config.WorkingDir.ValueString() | |||
|
|||
cmd := exec.CommandContext(ctx, filteredProgram[0], filteredProgram[1:]...) | |||
if errors.Is(cmd.Err, exec.ErrDot) { |
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.
I'm unsure how to test this.
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.
Hacky idea incoming, maybe it'll spur out a better idea 😆 :
- Instead of
go install
-ing the test program, you couldgo build
the test program into the working directory - Then update the
PATH
variable to include the working directoryPATH=./
witht.Setenv
- Execute the provider test with just the binary name
tf-acc-external-data-source
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.
Thanks for the suggestion.
I've tried placing a copy of the tf-acc-external-data-source
binary in /internal/provider
. The following test passes when the workaround code is not in place:
func TestDataSource_CurrentDir(t *testing.T) {
wd, err := os.Getwd()
if err != nil {
t.Error(err)
}
p := os.Getenv("PATH")
t.Setenv("PATH", fmt.Sprintf("%s:%s", p, wd))
resource.UnitTest(t, resource.TestCase{
ProtoV5ProviderFactories: protoV5ProviderFactories(),
Steps: []resource.TestStep{
{
Config: fmt.Sprintf(`
data "external" "test" {
program = [%[1]q]
query = {
value = null,
value2 = ""
}
}
`, "./tf-acc-external-data-source"),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("data.external.test", "result.value", ""),
resource.TestCheckResourceAttr("data.external.test", "result.value2", ""),
),
},
},
})
}
Similarly, the test passes when using the following config:
Config: fmt.Sprintf(`
data "external" "test" {
program = [%[1]q]
query = {
value = null,
value2 = ""
}
}
`, "tf-acc-external-data-source"),
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.
I believe to trigger the ErrDot
, you want the resulting path that finds the executable to be a relative path, that snippet looks like the resulting path would be absolute from os.Getwd()
. Try something like this:
func TestDataSource_CurrentDir(t *testing.T) {
p := os.Getenv("PATH")
t.Setenv("PATH", fmt.Sprintf("%s:./", p))
resource.UnitTest(t, resource.TestCase{
ProtoV5ProviderFactories: protoV5ProviderFactories(),
Steps: []resource.TestStep{
{
Config: fmt.Sprintf(`
data "external" "test" {
program = [%[1]q]
query = {
value = null,
value2 = ""
}
}
`, "tf-acc-external-data-source"),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("data.external.test", "result.value", ""),
resource.TestCheckResourceAttr("data.external.test", "result.value2", ""),
),
},
},
})
}
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.
Prior to adding workaround code:
_, err = exec.LookPath(filteredProgram[0])
if errors.Is(err, exec.ErrDot) {
err = nil
}
/* ... */
cmd := exec.CommandContext(ctx, filteredProgram[0], filteredProgram[1:]...)
if errors.Is(cmd.Err, exec.ErrDot) {
cmd.Err = nil
}
TestDataSource_CurrentDir
test fails with:
Error: External Program Lookup Failed
with data.external.test,
on terraform_plugin_test.tf line 3, in data "external" "test":
3: program = ["tf-acc-external-data-source"]
The data source received an unexpected error while attempting to parse the
query. The data source received an unexpected error while attempting to find
the program.
The program must be accessible according to the platform where Terraform is
running.
If the expected program should be automatically found on the platform where
Terraform is running, ensure that the program is in an expected directory. On
Unix-based platforms, these directories are typically searched based on the
'$PATH' environment variable. On Windows-based platforms, these directories
are typically searched based on the '%PATH%' environment variable.
If the expected program is relative to the Terraform configuration, it is
recommended that the program name includes the interpolated value of
'path.module' before the program name to ensure that it is compatible with
varying module usage. For example: "${path.module}/my-program"
The program must also be executable according to the platform where Terraform
is running. On Unix-based platforms, the file on the filesystem must have the
executable bit set. On Windows-based platforms, no action is typically
necessary.
Platform: darwin
Program: "tf-acc-external-data-source"
Error: exec: "tf-acc-external-data-source": cannot run executable found
relative to current directory
From the docs, looks like the
|
Another drive-by thought, do we have to worry about |
There is an issue for Fix Acceptance Tests on Windows which we can use for tracking. |
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.
Looks good to me 🚀 It might also be good to create a followup issue for removing the workaround and adding code comments pointing to that issue. 👍
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Closes: #191