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

Terraform working directory not set to ConfigDirectory when in use #277

Open
austinvalle opened this issue Jan 16, 2024 · 1 comment
Open
Labels
bug Something isn't working

Comments

@austinvalle
Copy link
Member

terraform-plugin-testing version

github.com/hashicorp/terraform-plugin-testing v1.6.0

The following code was from the local provider when attempting to refactor some of the tests to use ConfigDirectory: https://github.com/hashicorp/terraform-provider-local/blob/3b59b2e8a560c88b572fb46d3965bc9ae3a52ff3/internal/provider/data_source_local_file_test.go

Problem

Implementing a test utilizing (TestStep).ConfigDirectory with the following directory structure:

 $ tree
.
├── data_source_local_file.go
├── data_source_local_file_test.go
├── .... other files
└── testdata
    └── TestLocalFileDataSource
        ├── local_file
        ├── main.tf

data_source_local_file_test.go

func TestLocalFileDataSource(t *testing.T) {
	content := "This is some content"
	checkSums := genFileChecksums([]byte(content))

	resource.UnitTest(t, resource.TestCase{
		ProtoV5ProviderFactories: protoV5ProviderFactories(),
		Steps: []resource.TestStep{
			{
				ConfigDirectory: config.TestNameDirectory(),
				Check: resource.ComposeAggregateTestCheckFunc(
					resource.TestCheckResourceAttr("data.local_file.file", "content", content),
					resource.TestCheckResourceAttr("data.local_file.file", "content_base64", base64.StdEncoding.EncodeToString([]byte(content))),
					resource.TestCheckResourceAttr("data.local_file.file", "content_md5", checkSums.md5Hex),
					resource.TestCheckResourceAttr("data.local_file.file", "content_sha1", checkSums.sha1Hex),
					resource.TestCheckResourceAttr("data.local_file.file", "content_sha256", checkSums.sha256Hex),
					resource.TestCheckResourceAttr("data.local_file.file", "content_base64sha256", checkSums.sha256Base64),
					resource.TestCheckResourceAttr("data.local_file.file", "content_sha512", checkSums.sha512Hex),
					resource.TestCheckResourceAttr("data.local_file.file", "content_base64sha512", checkSums.sha512Base64),
				),
			},
		},
	})
}

When attempting to reference the local_file file from the terraform config directory, you can't use a relative reference, as the working directory is not set to testdata/TestLocalFileDataSource.

main.tf (error)

data "local_file" "file" {
  # This doesn't work, throws an error saying the file can't be found
  filename = "${path.module}/local_file"
}

main.tf (success)

data "local_file" "file" {
  # This works
  filename = "${path.module}/testdata/TestLocalFileDataSource/local_file"
}

Proposal

We should copy the contents of ConfigDirectory to the root of the working directory that terraform will run in. This may have compatibility concerns? Open to other suggestions.

At the very least, we should document that this behavior exists if we opt not to fix it.

@austinvalle austinvalle added the bug Something isn't working label Jan 16, 2024
@bflad
Copy link
Contributor

bflad commented Jan 16, 2024

Just to provide context on the issue title wording: we intentionally use a temporary working directory to not potentially leave testing-caused files in provider codebase. If I recall, we also wanted ConfigDirectory to copy the running test directory sub-directories (e.g. the top level testdata in your example) similar to how it worked with the prior Config field, so developers could migrate between the two without necessarily changing the Terraform configuration. I think both of these behaviors are desirable in the testing logic over potentially using the ConfigDirectory as the working directory directly.

I do personally think it would be nice if we did copy all files/directories from ConfigDirectory to the temporary working directory to do exactly as you propose though, however, there may be a wrinkle here if there are duplicate directory names between the running test directory sub-directories and ConfigDirectory sub-directories (e.g. if both had their own testdata directories in your example). Maybe pretty uncommon, but the testing logic should probably do something in that situation (e.g. error) rather than picking one or something. Off the top of head, I'm not sure if there are other compatibility concerns if we do wind up copying the ConfigDirectory files/sub-directories.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants