From 7f68a8a3c1bb782bd1b0cf6ae68d5c52a841dd08 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Tue, 4 Apr 2023 10:04:25 +0100 Subject: [PATCH 1/3] Fix path used when persisting working directory (#55) --- helper/resource/testing_new.go | 5 ++-- helper/resource/teststep_providers_test.go | 34 +++++----------------- internal/plugintest/helper.go | 4 +++ internal/plugintest/util.go | 19 ++++++++++-- internal/plugintest/util_test.go | 2 +- internal/plugintest/working_dir.go | 4 +++ 6 files changed, 36 insertions(+), 32 deletions(-) diff --git a/helper/resource/testing_new.go b/helper/resource/testing_new.go index d671a0a6f..66b7fd2c5 100644 --- a/helper/resource/testing_new.go +++ b/helper/resource/testing_new.go @@ -460,11 +460,10 @@ func copyWorkingDir(ctx context.Context, t testing.T, stepNumber int, wd *plugin } workingDir := wd.GetHelper().WorkingDirectory() - parentDir := filepath.Dir(workingDir) - dest := parentDir + "_" + strconv.Itoa(stepNumber) + dest := filepath.Join(workingDir, fmt.Sprintf("%s%s", "step_", strconv.Itoa(stepNumber))) - err := plugintest.CopyDir(wd.GetHelper().WorkingDirectory(), dest) + err := plugintest.CopyDir(workingDir, dest, stepNumber) if err != nil { logging.HelperResourceError(ctx, "Unexpected error copying working directory files", diff --git a/helper/resource/teststep_providers_test.go b/helper/resource/teststep_providers_test.go index 986cf7b0c..2f4f5c9fa 100644 --- a/helper/resource/teststep_providers_test.go +++ b/helper/resource/teststep_providers_test.go @@ -1819,10 +1819,8 @@ func TestTest_TestStep_ProviderFactories_Import_External_WithPersistMatch_WithPe Steps: testSteps, }) - workingDirPath := filepath.Dir(workingDir) - for testStepIndex := range testSteps { - dir := workingDirPath + "_" + strconv.Itoa(testStepIndex+1) + dir := filepath.Join(workingDir, fmt.Sprintf("step_%s", strconv.Itoa(testStepIndex+1))) dirEntries, err := os.ReadDir(dir) if err != nil { @@ -1929,10 +1927,8 @@ func TestTest_TestStep_ProviderFactories_Import_External_WithoutPersistNonMatch_ Steps: testSteps, }) - workingDirPath := filepath.Dir(workingDir) - for testStepIndex := range testSteps { - dir := workingDirPath + "_" + strconv.Itoa(testStepIndex+1) + dir := filepath.Join(workingDir, fmt.Sprintf("step_%s", strconv.Itoa(testStepIndex+1))) dirEntries, err := os.ReadDir(dir) if err != nil { @@ -1949,23 +1945,11 @@ func TestTest_TestStep_ProviderFactories_Import_External_WithoutPersistNonMatch_ } } - configPlanStateFiles := []string{ - "terraform_plugin_test.tf", - "terraform.tfstate", - "tfplan", - } - - for _, file := range configPlanStateFiles { - // Skip verifying state and plan for first test step as ImportStatePersist is - // false so the state is not persisted and there is no plan file if the - // resource does not already exist. - if testStepIndex == 0 && (file == "terraform.tfstate" || file == "tfplan") { - break - } - _, err = os.Stat(filepath.Join(workingDirName, file)) - if err != nil { - t.Errorf("cannot stat %s in %s: %s", file, workingDirName, err) - } + // Only stat the terraform_plugin_test.tf file as 2 working directories are + // created because import state is not persisted. + _, err = os.Stat(filepath.Join(workingDirName, "terraform_plugin_test.tf")) + if err != nil { + t.Errorf("cannot stat %s in %s: %s", "terraform_plugin_test.tf", workingDirName, err) } } } @@ -2075,10 +2059,8 @@ func TestTest_TestStep_ProviderFactories_CopyWorkingDir_EachTestStep(t *testing. Steps: testSteps, }) - workingDirPath := filepath.Dir(workingDir) - for k := range testSteps { - dir := workingDirPath + "_" + strconv.Itoa(k+1) + dir := filepath.Join(workingDir, fmt.Sprintf("step_%s", strconv.Itoa(k+1))) _, err := os.ReadDir(dir) if err != nil { diff --git a/internal/plugintest/helper.go b/internal/plugintest/helper.go index 04b12fe9c..ef14a0596 100644 --- a/internal/plugintest/helper.go +++ b/internal/plugintest/helper.go @@ -92,6 +92,10 @@ func InitHelper(ctx context.Context, config *Config) (*Helper, error) { // Call this before returning from TestMain to minimize the amount of detritus // left behind in the filesystem after the tests complete. func (h *Helper) Close() error { + if os.Getenv(EnvTfAccPersistWorkingDir) != "" { + return nil + } + if h.execTempDir != "" { err := os.RemoveAll(h.execTempDir) if err != nil { diff --git a/internal/plugintest/util.go b/internal/plugintest/util.go index bd1d00abb..19bdf9cd7 100644 --- a/internal/plugintest/util.go +++ b/internal/plugintest/util.go @@ -9,6 +9,8 @@ import ( "os" "path" "path/filepath" + "strconv" + "strings" ) func symlinkFile(src string, dest string) error { @@ -100,7 +102,7 @@ func CopyFile(src, dest string) error { // CopyDir recursively copies directories and files // from src to dest. -func CopyDir(src string, dest string) error { +func CopyDir(src, dest string, stepNumber int) error { srcInfo, err := os.Stat(src) if err != nil { return fmt.Errorf("unable to stat: %w", err) @@ -119,8 +121,21 @@ func CopyDir(src string, dest string) error { srcFilepath := path.Join(src, dirEntry.Name()) destFilepath := path.Join(dest, dirEntry.Name()) + if srcFilepath == dest { + continue + } + + for x := 1; x <= stepNumber; x++ { + destWithoutStepNumber := strings.TrimRight(dest, strconv.Itoa(stepNumber)) + destWithNumber := destWithoutStepNumber + strconv.Itoa(x) + + if destWithNumber == srcFilepath { + continue + } + } + if dirEntry.IsDir() { - if err = CopyDir(srcFilepath, destFilepath); err != nil { + if err = CopyDir(srcFilepath, destFilepath, stepNumber); err != nil { return fmt.Errorf("unable to copy directory: %w", err) } } else { diff --git a/internal/plugintest/util_test.go b/internal/plugintest/util_test.go index 58c0dc12f..ce1c05829 100644 --- a/internal/plugintest/util_test.go +++ b/internal/plugintest/util_test.go @@ -50,7 +50,7 @@ func TestCopyDir(t *testing.T) { t.Fatalf("cannot create src file: %s", err) } - err = CopyDir(srcDir, srcDir+"_1") + err = CopyDir(srcDir, srcDir+"_1", 1) if err != nil { t.Fatalf("cannot copy dir: %s", err) } diff --git a/internal/plugintest/working_dir.go b/internal/plugintest/working_dir.go index 734b6c9b5..8caa15720 100644 --- a/internal/plugintest/working_dir.go +++ b/internal/plugintest/working_dir.go @@ -51,6 +51,10 @@ type WorkingDir struct { // working directory. After this method is called, the working directory object // is invalid and may no longer be used. func (wd *WorkingDir) Close() error { + if os.Getenv(EnvTfAccPersistWorkingDir) != "" { + return nil + } + return os.RemoveAll(wd.baseDir) } From 77bb5328e44c8008337eca1b5230e2db4f0e8f1b Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Tue, 4 Apr 2023 10:08:59 +0100 Subject: [PATCH 2/3] Adding CHANGELOG entry (#55) --- .changes/unreleased/BUG FIXES-20230404-100828.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/unreleased/BUG FIXES-20230404-100828.yaml diff --git a/.changes/unreleased/BUG FIXES-20230404-100828.yaml b/.changes/unreleased/BUG FIXES-20230404-100828.yaml new file mode 100644 index 000000000..65b69ab7f --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20230404-100828.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'helper/resource: Fix path used when persisting working directory' +time: 2023-04-04T10:08:28.3828+01:00 +custom: + Issue: "113" From 346995641c22c6c00531c5153ffb4fd5e1cdda5f Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Tue, 4 Apr 2023 14:03:14 +0100 Subject: [PATCH 3/3] Using root of working dir to determine which files and directories to copy (#55) --- helper/resource/testing_new.go | 5 ++++- helper/resource/teststep_providers_test.go | 22 +++++++++++++++++----- internal/plugintest/util.go | 16 +++------------- internal/plugintest/util_test.go | 2 +- internal/plugintest/working_dir.go | 5 +++++ 5 files changed, 30 insertions(+), 20 deletions(-) diff --git a/helper/resource/testing_new.go b/helper/resource/testing_new.go index 66b7fd2c5..da4785f0a 100644 --- a/helper/resource/testing_new.go +++ b/helper/resource/testing_new.go @@ -463,7 +463,10 @@ func copyWorkingDir(ctx context.Context, t testing.T, stepNumber int, wd *plugin dest := filepath.Join(workingDir, fmt.Sprintf("%s%s", "step_", strconv.Itoa(stepNumber))) - err := plugintest.CopyDir(workingDir, dest, stepNumber) + baseDir := wd.BaseDir() + rootBaseDir := strings.TrimLeft(baseDir, workingDir) + + err := plugintest.CopyDir(workingDir, dest, rootBaseDir) if err != nil { logging.HelperResourceError(ctx, "Unexpected error copying working directory files", diff --git a/helper/resource/teststep_providers_test.go b/helper/resource/teststep_providers_test.go index 2f4f5c9fa..44960abc0 100644 --- a/helper/resource/teststep_providers_test.go +++ b/helper/resource/teststep_providers_test.go @@ -1945,11 +1945,23 @@ func TestTest_TestStep_ProviderFactories_Import_External_WithoutPersistNonMatch_ } } - // Only stat the terraform_plugin_test.tf file as 2 working directories are - // created because import state is not persisted. - _, err = os.Stat(filepath.Join(workingDirName, "terraform_plugin_test.tf")) - if err != nil { - t.Errorf("cannot stat %s in %s: %s", "terraform_plugin_test.tf", workingDirName, err) + configPlanStateFiles := []string{ + "terraform_plugin_test.tf", + "terraform.tfstate", + "tfplan", + } + + for _, file := range configPlanStateFiles { + // Skip verifying state and plan for first test step as ImportStatePersist is + // false so the state is not persisted and there is no plan file if the + // resource does not already exist. + if testStepIndex == 0 && (file == "terraform.tfstate" || file == "tfplan") { + break + } + _, err = os.Stat(filepath.Join(workingDirName, file)) + if err != nil { + t.Errorf("cannot stat %s in %s: %s", file, workingDirName, err) + } } } } diff --git a/internal/plugintest/util.go b/internal/plugintest/util.go index 19bdf9cd7..067dc74af 100644 --- a/internal/plugintest/util.go +++ b/internal/plugintest/util.go @@ -9,7 +9,6 @@ import ( "os" "path" "path/filepath" - "strconv" "strings" ) @@ -102,7 +101,7 @@ func CopyFile(src, dest string) error { // CopyDir recursively copies directories and files // from src to dest. -func CopyDir(src, dest string, stepNumber int) error { +func CopyDir(src, dest, baseDirName string) error { srcInfo, err := os.Stat(src) if err != nil { return fmt.Errorf("unable to stat: %w", err) @@ -121,21 +120,12 @@ func CopyDir(src, dest string, stepNumber int) error { srcFilepath := path.Join(src, dirEntry.Name()) destFilepath := path.Join(dest, dirEntry.Name()) - if srcFilepath == dest { + if !strings.Contains(srcFilepath, baseDirName) { continue } - for x := 1; x <= stepNumber; x++ { - destWithoutStepNumber := strings.TrimRight(dest, strconv.Itoa(stepNumber)) - destWithNumber := destWithoutStepNumber + strconv.Itoa(x) - - if destWithNumber == srcFilepath { - continue - } - } - if dirEntry.IsDir() { - if err = CopyDir(srcFilepath, destFilepath, stepNumber); err != nil { + if err = CopyDir(srcFilepath, destFilepath, baseDirName); err != nil { return fmt.Errorf("unable to copy directory: %w", err) } } else { diff --git a/internal/plugintest/util_test.go b/internal/plugintest/util_test.go index ce1c05829..ceb3badc0 100644 --- a/internal/plugintest/util_test.go +++ b/internal/plugintest/util_test.go @@ -50,7 +50,7 @@ func TestCopyDir(t *testing.T) { t.Fatalf("cannot create src file: %s", err) } - err = CopyDir(srcDir, srcDir+"_1", 1) + err = CopyDir(srcDir, srcDir+"_1", "") if err != nil { t.Fatalf("cannot copy dir: %s", err) } diff --git a/internal/plugintest/working_dir.go b/internal/plugintest/working_dir.go index 8caa15720..9ee2046f2 100644 --- a/internal/plugintest/working_dir.go +++ b/internal/plugintest/working_dir.go @@ -47,6 +47,11 @@ type WorkingDir struct { reattachInfo tfexec.ReattachInfo } +// BaseDir returns the path to the root of the working directory tree. +func (wd *WorkingDir) BaseDir() string { + return wd.baseDir +} + // Close deletes the directories and files created to represent the receiving // working directory. After this method is called, the working directory object // is invalid and may no longer be used.