From d6889103603e31d0735428c7104669234581e2d0 Mon Sep 17 00:00:00 2001 From: Matthew Riley Date: Tue, 9 Jan 2018 23:22:35 -0800 Subject: [PATCH 1/2] Remove named parameters from CopyTestToTemporaryDirectory Fix two references to the now-unnamed `dir` that should have been to other variables. Check a real condition before the deferred call to RemoveAll instead of checking the error return. --- pkg/testing/integration/program.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/testing/integration/program.go b/pkg/testing/integration/program.go index d6e28d44a..01b914d02 100644 --- a/pkg/testing/integration/program.go +++ b/pkg/testing/integration/program.go @@ -644,11 +644,11 @@ func performExtraRuntimeValidation( } // CopyTestToTemporaryDirectory creates a temporary directory to run the test in and copies the test to it. -func CopyTestToTemporaryDirectory(t *testing.T, opts *ProgramTestOptions) (dir string, err error) { +func CopyTestToTemporaryDirectory(t *testing.T, opts *ProgramTestOptions) (string, error) { // Ensure the required programs are present. if opts.Bin == "" { var pulumi string - pulumi, err = exec.LookPath("pulumi") + pulumi, err := exec.LookPath("pulumi") if err != nil { return "", errors.Wrapf(err, "Expected to find `pulumi` binary on $PATH") } @@ -656,7 +656,7 @@ func CopyTestToTemporaryDirectory(t *testing.T, opts *ProgramTestOptions) (dir s } if opts.YarnBin == "" { var yarn string - yarn, err = exec.LookPath("yarn") + yarn, err := exec.LookPath("yarn") if err != nil { return "", errors.Wrapf(err, "Expected to find `yarn` binary on $PATH") } @@ -683,7 +683,7 @@ func CopyTestToTemporaryDirectory(t *testing.T, opts *ProgramTestOptions) (dir s opts.Stderr = stderr } - pioutil.MustFprintf(opts.Stdout, "sample: %v\n", dir) + pioutil.MustFprintf(opts.Stdout, "sample: %v\n", sourceDir) pioutil.MustFprintf(opts.Stdout, "pulumi: %v\n", opts.Bin) pioutil.MustFprintf(opts.Stdout, "yarn: %v\n", opts.YarnBin) @@ -694,23 +694,25 @@ func CopyTestToTemporaryDirectory(t *testing.T, opts *ProgramTestOptions) (dir s } // Clean up the temporary directory on failure + deleteTargetDir := true defer func() { - if err != nil { + if deleteTargetDir { contract.IgnoreError(os.RemoveAll(targetDir)) } }() // Copy the source project - if copyerr := fsutil.CopyFile(targetDir, sourceDir, nil); copyerr != nil { - return "", copyerr + if err = fsutil.CopyFile(targetDir, sourceDir, nil); err != nil { + return "", err } err = prepareProject(t, opts, targetDir) if err != nil { - return "", errors.Wrapf(err, "Failed to prepare %v", dir) + return "", errors.Wrapf(err, "Failed to prepare %v", targetDir) } pioutil.MustFprintf(stdout, "projdir: %v\n", targetDir) + deleteTargetDir = false return targetDir, nil } From 047a6c3c568de2509e262ce07498ad1b5985f8aa Mon Sep 17 00:00:00 2001 From: Matthew Riley Date: Tue, 9 Jan 2018 23:25:49 -0800 Subject: [PATCH 2/2] Make "delete temp dir" condition explicit Previously we checked if the test had failed -- but the call to `assert.NoError` that actually fails the test sometimes hasn't happened yet. --- pkg/testing/integration/program.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/pkg/testing/integration/program.go b/pkg/testing/integration/program.go index 01b914d02..64f00af6b 100644 --- a/pkg/testing/integration/program.go +++ b/pkg/testing/integration/program.go @@ -304,17 +304,18 @@ func testLifeCycleInitAndDestroy( return errors.Wrap(err, "Couldn't copy test to temporary directory") } + // Keep the temporary test directory around for debugging unless + // the test completes successfully. + keepTestDir := true defer func() { - if t.Failed() { - // Test failed -- keep temporary files around for debugging. - // Maybe also copy to "failed tests" directory. + if keepTestDir { + // Maybe copy to "failed tests" directory. failedTestsDir := os.Getenv("PULUMI_FAILED_TESTS_DIR") if failedTestsDir != "" { dest := filepath.Join(failedTestsDir, t.Name()+uniqueSuffix()) contract.IgnoreError(fsutil.CopyFile(dest, dir, nil)) } } else { - // Test passed -- delete temporary files. contract.IgnoreError(os.RemoveAll(dir)) } }() @@ -332,7 +333,13 @@ func testLifeCycleInitAndDestroy( } }() - return between(t, opts, dir) + err = between(t, opts, dir) + if err != nil { + return err + } + + keepTestDir = false + return nil } func testLifeCycleInitialize(t *testing.T, opts *ProgramTestOptions, dir string) error {