From 14d50aaa50eec11ff4e21e8a25f1774ddb704f47 Mon Sep 17 00:00:00 2001 From: Nick Date: Fri, 14 Aug 2020 17:32:45 -0700 Subject: [PATCH] [Robustness] Fix for kopia runner and custom working directory (#533) * [Robustness] Fix for kopia runner and custom work dir Apply fix similar to #293 for the robustness kopia runner. Add control for runner working directory. --- tests/robustness/engine/engine.go | 14 +++++---- tests/robustness/engine/engine_test.go | 12 ++++---- tests/robustness/robustness_test/main_test.go | 2 +- tests/robustness/snapmeta/kopia_meta.go | 6 ++-- tests/tools/kopiarunner/kopia_snapshotter.go | 4 +-- tests/tools/kopiarunner/kopiarun.go | 30 +++++-------------- tests/tools/kopiarunner/kopiarun_test.go | 4 ++- 7 files changed, 31 insertions(+), 41 deletions(-) diff --git a/tests/robustness/engine/engine.go b/tests/robustness/engine/engine.go index 35f8467cf..1b5bdaf8e 100644 --- a/tests/robustness/engine/engine.go +++ b/tests/robustness/engine/engine.go @@ -6,6 +6,7 @@ import ( "context" "fmt" + "io/ioutil" "math/rand" "os" "strconv" @@ -41,10 +42,13 @@ type Engine struct { // - Kopia test repo snapshotter // - Kopia metadata storage repo // - FSWalker data integrity checker. -func NewEngine() (*Engine, error) { - e := new(Engine) +func NewEngine(workingDir string) (*Engine, error) { + baseDirPath, err := ioutil.TempDir(workingDir, "engine-data-") + if err != nil { + return nil, err + } - var err error + e := new(Engine) // Fill the file writer e.FileWriter, err = fio.NewRunner() @@ -56,7 +60,7 @@ func NewEngine() (*Engine, error) { e.cleanupRoutines = append(e.cleanupRoutines, e.FileWriter.Cleanup) // Fill Snapshotter interface - kopiaSnapper, err := kopiarunner.NewKopiaSnapshotter() + kopiaSnapper, err := kopiarunner.NewKopiaSnapshotter(baseDirPath) if err != nil { e.Cleanup() //nolint:errcheck return nil, err @@ -66,7 +70,7 @@ func NewEngine() (*Engine, error) { e.TestRepo = kopiaSnapper // Fill the snapshot store interface - snapStore, err := snapmeta.New() + snapStore, err := snapmeta.New(baseDirPath) if err != nil { e.Cleanup() //nolint:errcheck return nil, err diff --git a/tests/robustness/engine/engine_test.go b/tests/robustness/engine/engine_test.go index 368c46bcf..4718891ba 100644 --- a/tests/robustness/engine/engine_test.go +++ b/tests/robustness/engine/engine_test.go @@ -26,7 +26,7 @@ ) func TestEngineWritefilesBasicFS(t *testing.T) { - eng, err := NewEngine() + eng, err := NewEngine("") if err == kopiarunner.ErrExeVariableNotSet || errors.Is(err, fio.ErrEnvNotSet) { t.Skip(err) } @@ -65,7 +65,7 @@ func TestEngineWritefilesBasicFS(t *testing.T) { } func TestWriteFilesBasicS3(t *testing.T) { - eng, err := NewEngine() + eng, err := NewEngine("") if err == kopiarunner.ErrExeVariableNotSet || errors.Is(err, fio.ErrEnvNotSet) { t.Skip(err) } @@ -104,7 +104,7 @@ func TestWriteFilesBasicS3(t *testing.T) { } func TestDeleteSnapshotS3(t *testing.T) { - eng, err := NewEngine() + eng, err := NewEngine("") if err == kopiarunner.ErrExeVariableNotSet || errors.Is(err, fio.ErrEnvNotSet) { t.Skip(err) } @@ -144,7 +144,7 @@ func TestDeleteSnapshotS3(t *testing.T) { } func TestSnapshotVerificationFail(t *testing.T) { - eng, err := NewEngine() + eng, err := NewEngine("") if err == kopiarunner.ErrExeVariableNotSet || errors.Is(err, fio.ErrEnvNotSet) { t.Skip(err) } @@ -210,7 +210,7 @@ func TestDataPersistency(t *testing.T) { defer os.RemoveAll(tempDir) - eng, err := NewEngine() + eng, err := NewEngine("") if err == kopiarunner.ErrExeVariableNotSet || errors.Is(err, fio.ErrEnvNotSet) { t.Skip(err) } @@ -251,7 +251,7 @@ func TestDataPersistency(t *testing.T) { testenv.AssertNoError(t, err) // Create a new engine - eng2, err := NewEngine() + eng2, err := NewEngine("") testenv.AssertNoError(t, err) defer eng2.cleanup() diff --git a/tests/robustness/robustness_test/main_test.go b/tests/robustness/robustness_test/main_test.go index d2435e74f..e80eb29cb 100644 --- a/tests/robustness/robustness_test/main_test.go +++ b/tests/robustness/robustness_test/main_test.go @@ -26,7 +26,7 @@ func TestMain(m *testing.M) { var err error - eng, err = engine.NewEngine() + eng, err = engine.NewEngine("") if err != nil { log.Println("skipping robustness tests:", err) diff --git a/tests/robustness/snapmeta/kopia_meta.go b/tests/robustness/snapmeta/kopia_meta.go index d04072787..9bf76d5bc 100644 --- a/tests/robustness/snapmeta/kopia_meta.go +++ b/tests/robustness/snapmeta/kopia_meta.go @@ -20,13 +20,13 @@ type kopiaMetadata struct { } // New instantiates a new Persister and returns it. -func New() (Persister, error) { - localDir, err := ioutil.TempDir("", "kopia-local-metadata-") +func New(baseDir string) (Persister, error) { + localDir, err := ioutil.TempDir(baseDir, "kopia-local-metadata-") if err != nil { return nil, err } - snap, err := kopiarunner.NewKopiaSnapshotter() + snap, err := kopiarunner.NewKopiaSnapshotter(localDir) if err != nil { return nil, err } diff --git a/tests/tools/kopiarunner/kopia_snapshotter.go b/tests/tools/kopiarunner/kopia_snapshotter.go index 0a946810f..fd6e72d91 100644 --- a/tests/tools/kopiarunner/kopia_snapshotter.go +++ b/tests/tools/kopiarunner/kopia_snapshotter.go @@ -17,8 +17,8 @@ type KopiaSnapshotter struct { } // NewKopiaSnapshotter instantiates a new KopiaSnapshotter and returns its pointer. -func NewKopiaSnapshotter() (*KopiaSnapshotter, error) { - runner, err := NewRunner() +func NewKopiaSnapshotter(baseDir string) (*KopiaSnapshotter, error) { + runner, err := NewRunner(baseDir) if err != nil { return nil, err } diff --git a/tests/tools/kopiarunner/kopiarun.go b/tests/tools/kopiarunner/kopiarun.go index 5f658ec5b..f3148880a 100644 --- a/tests/tools/kopiarunner/kopiarun.go +++ b/tests/tools/kopiarunner/kopiarun.go @@ -2,6 +2,7 @@ package kopiarunner import ( + "bytes" "errors" "io/ioutil" "log" @@ -9,7 +10,6 @@ "os/exec" "path/filepath" "strings" - "sync" ) const ( @@ -28,13 +28,13 @@ type Runner struct { var ErrExeVariableNotSet = errors.New("KOPIA_EXE variable has not been set") // NewRunner initializes a new kopia runner and returns its pointer. -func NewRunner() (*Runner, error) { +func NewRunner(baseDir string) (*Runner, error) { exe := os.Getenv("KOPIA_EXE") if exe == "" { return nil, ErrExeVariableNotSet } - configDir, err := ioutil.TempDir("", "kopia-config") + configDir, err := ioutil.TempDir(baseDir, "kopia-config") if err != nil { return nil, err } @@ -69,28 +69,12 @@ func (kr *Runner) Run(args ...string) (stdout, stderr string, err error) { c := exec.Command(kr.Exe, cmdArgs...) c.Env = append(os.Environ(), kr.environment...) - stderrPipe, err := c.StderrPipe() - if err != nil { - return stdout, stderr, err - } - - var errOut []byte - - var wg sync.WaitGroup - - wg.Add(1) - - go func() { - defer wg.Done() - - errOut, err = ioutil.ReadAll(stderrPipe) - }() + errOut := &bytes.Buffer{} + c.Stderr = errOut o, err := c.Output() - wg.Wait() + log.Printf("finished '%s %v' with err=%v and output:\nSTDOUT:\n%v\nSTDERR:\n%v", kr.Exe, argsStr, err, string(o), errOut.String()) - log.Printf("finished '%s %v' with err=%v and output:\n%v\n%v", kr.Exe, argsStr, err, string(o), string(errOut)) - - return string(o), string(errOut), err + return string(o), errOut.String(), err } diff --git a/tests/tools/kopiarunner/kopiarun_test.go b/tests/tools/kopiarunner/kopiarun_test.go index fe1341281..996d4d6fe 100644 --- a/tests/tools/kopiarunner/kopiarun_test.go +++ b/tests/tools/kopiarunner/kopiarun_test.go @@ -61,7 +61,7 @@ func TestKopiaRunner(t *testing.T) { t.Fatal("Unable to set environment variable KOPIA_EXE") } - runner, err := NewRunner() + runner, err := NewRunner("") if (err != nil) != tt.expNewRunnerErr { t.Fatalf("Expected NewRunner error: %v, got %v", tt.expNewRunnerErr, err) } @@ -70,6 +70,8 @@ func TestKopiaRunner(t *testing.T) { continue } + defer runner.Cleanup() + _, _, err = runner.Run(tt.args...) if (err != nil) != tt.expRunErr { t.Fatalf("Expected Run error: %v, got %v", tt.expRunErr, err)