From 55c9ff680151bf171cd6dc3747fda97b9a7b1aa1 Mon Sep 17 00:00:00 2001 From: Dustin Long Date: Fri, 6 Nov 2020 09:37:06 -0500 Subject: [PATCH] fix(init): TargetDir for init, can be absolute, is created if needed Prior to this change, lib.InitDataset would take two parameters, Dir which would need to exist already, and Mkdir which would be created but could only be a single directory name instead of a relative path. Fix this problem, and also add multiple tests, and cleanup some TODOs. --- api/fsi.go | 17 ++--- api/fsi_test.go | 6 +- cmd/fsi_integration_test.go | 37 +++++++++++ cmd/init.go | 62 ++++++++---------- cmd/test_runner_test.go | 1 + fsi/init.go | 106 +++++++++++++++--------------- fsi/init_test.go | 62 +++++++++++++++++- fsi/status_test.go | 24 +++---- lib/datasets_test.go | 26 ++++---- lib/fsi.go | 25 +++++--- lib/fsi_test.go | 125 ++++++++++++++++++++++++++++++++++-- lib/test_runner_test.go | 14 ++-- 12 files changed, 355 insertions(+), 150 deletions(-) diff --git a/api/fsi.go b/api/fsi.go index 749a1a76c..139bc1744 100644 --- a/api/fsi.go +++ b/api/fsi.go @@ -142,18 +142,11 @@ func (h *FSIHandlers) InitHandler(routePrefix string) http.HandlerFunc { func (h *FSIHandlers) initHandler(routePrefix string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - // Backwards compatibility shim for now, can use either "dir" or "filepath". - // TODO: Update desktop to always use "dir", delete "filepath". - dir := r.FormValue("dir") - if dir == "" && r.FormValue("filepath") != "" { - dir = r.FormValue("filepath") - } - p := &lib.InitFSIDatasetParams{ - Dir: dir, - Name: r.FormValue("name"), - Format: r.FormValue("format"), - Mkdir: r.FormValue("mkdir"), - SourceBodyPath: r.FormValue("sourcebodypath"), + p := &lib.InitDatasetParams{ + TargetDir: r.FormValue("targetdir"), + Name: r.FormValue("name"), + Format: r.FormValue("format"), + BodyPath: r.FormValue("bodypath"), } var name string diff --git a/api/fsi_test.go b/api/fsi_test.go index f33295f6a..5162b0d53 100644 --- a/api/fsi_test.go +++ b/api/fsi_test.go @@ -98,9 +98,9 @@ func TestNoHistory(t *testing.T) { // Create a linked dataset without saving, it has no versions in the repository ref, err := run.Inst.FSI().InitDataset(fsi.InitParams{ - Dir: workDir, - Name: "test_ds", - Format: "csv", + TargetDir: workDir, + Name: "test_ds", + Format: "csv", }) if err != nil { t.Fatal(err) diff --git a/cmd/fsi_integration_test.go b/cmd/fsi_integration_test.go index 5d5846b2f..71a229573 100644 --- a/cmd/fsi_integration_test.go +++ b/cmd/fsi_integration_test.go @@ -276,6 +276,22 @@ func TestInitExplicitDirectory(t *testing.T) { } } +// Test init command can use an explicit directory +func TestInitExplicitAbsoluteDirectory(t *testing.T) { + run := NewFSITestRunner(t, "test_peer_init_abs_dir", "qri_test_init_abs_dir") + defer run.Delete() + + absDir := filepath.Join(run.RootPath, "datasets/my_work_dir") + run.MustExec(t, fmt.Sprintf("qri init --name abs_dir --format csv %s", absDir)) + + // Verify the directory contains the files that we expect. + dirContents := listDirectory(absDir) + expectContents := []string{".qri-ref", "body.csv", "meta.json", "structure.json"} + if diff := cmp.Diff(expectContents, dirContents); diff != "" { + t.Errorf("directory contents (-want +got):\n%s", diff) + } +} + // Test init command can build dscache func TestInitDscache(t *testing.T) { run := NewFSITestRunner(t, "test_peer_init_dscache", "qri_test_init_dscache") @@ -376,6 +392,27 @@ func TestGetBodyWithoutStructure(t *testing.T) { } } +// Test init command accepts dataset name on standard input +func TestInitUsingStdin(t *testing.T) { + run := NewFSITestRunner(t, "test_peer_init_json_body", "qri_test_init_json_body") + defer run.Delete() + + workDir := run.CreateAndChdirToWorkDir("json_body") + + // Init with configuration from standard input + err := run.ExecCommandWithStdin(run.Context, "qri init", "my_dataset\njson") + if err != nil { + t.Fatal(err) + } + + // Verify the directory contains the files that we expect. + dirContents := listDirectory(workDir) + expectContents := []string{".qri-ref", "body.json", "meta.json", "structure.json"} + if diff := cmp.Diff(expectContents, dirContents); diff != "" { + t.Errorf("directory contents (-want +got):\n%s", diff) + } +} + // Test init command can create a json body using the format flag func TestInitForJsonBody(t *testing.T) { run := NewFSITestRunner(t, "test_peer_init_json_body", "qri_test_init_json_body") diff --git a/cmd/init.go b/cmd/init.go index 05f2ec901..88af84663 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -1,7 +1,6 @@ package cmd import ( - "os" "path/filepath" "strings" @@ -46,8 +45,8 @@ already existing body file using the 'body' flag.`, } cmd.Flags().StringVar(&o.Name, "name", "", "name of the dataset") - cmd.Flags().StringVar(&o.Format, "format", "", "format of dataset") - cmd.Flags().StringVar(&o.SourceBodyPath, "body", "", "path to the body file") + cmd.Flags().StringVar(&o.Format, "format", "", "format of dataset body") + cmd.Flags().StringVar(&o.BodyPath, "body", "", "path to the body file") cmd.Flags().BoolVarP(&o.UseDscache, "use-dscache", "", false, "experimental: build and use dscache if none exists") return cmd @@ -57,11 +56,16 @@ already existing body file using the 'body' flag.`, type InitOptions struct { ioes.IOStreams - Name string - Format string - SourceBodyPath string - Mkdir string - UseDscache bool + // Name of the dataset that will be created + Name string + // Format of the body + Format string + // Body file to initialize dataset with, if blank an example csv will be used + BodyPath string + // Path to use as a working directory. Will be created if it does not exist yet. + TargetDir string + // Experimental: Use dscache subsystem to store dataset references + UseDscache bool DatasetMethods *lib.DatasetMethods FSIMethods *lib.FSIMethods @@ -73,27 +77,18 @@ func (o *InitOptions) Complete(f Factory, args []string) (err error) { return err } o.FSIMethods, err = f.FSIMethods() - if len(args) > 0 && args[0] != "." { - o.Mkdir = args[0] + if len(args) > 0 { + o.TargetDir = args[0] } return err } // Run executes the `init` command func (o *InitOptions) Run() (err error) { - pwd, err := os.Getwd() - if err != nil { - return err - } - targetDir := pwd - if o.Mkdir != "" { - targetDir = o.Mkdir - } - // First, check if the directory can be init'd, before prompting for any input - canInitParams := lib.InitFSIDatasetParams{ - Dir: targetDir, - SourceBodyPath: o.SourceBodyPath, + canInitParams := lib.InitDatasetParams{ + TargetDir: o.TargetDir, + BodyPath: o.BodyPath, } if err = o.FSIMethods.CanInitDatasetWorkDir(&canInitParams, nil); err != nil { return err @@ -101,20 +96,18 @@ func (o *InitOptions) Run() (err error) { // Suggestion for the dataset name defaults to directory it is being linked into if o.Name == "" { - // TODO(dustmop): Currently all tests that call `init` use the --name flag. Add a test - // that receives stdin and checks what is written to stdout. - suggestedName := dsref.GenerateName(filepath.Base(targetDir), "dataset_") + suggestedName := dsref.GenerateName(filepath.Base(o.TargetDir), "dataset_") o.Name = inputText(o.Out, o.In, "Name of new dataset", suggestedName) } - // If user inputted there own dataset name, make sure it's valid. + // If user inputted their own dataset name, make sure it's valid. if err := dsref.EnsureValidName(o.Name); err != nil { return err } // If --body flag is set, use that to figure out the format - if o.SourceBodyPath != "" { - ext := filepath.Ext(o.SourceBodyPath) + if o.BodyPath != "" { + ext := filepath.Ext(o.BodyPath) if strings.HasPrefix(ext, ".") { ext = ext[1:] } @@ -125,13 +118,12 @@ func (o *InitOptions) Run() (err error) { o.Format = inputText(o.Out, o.In, "Format of dataset, csv or json", "csv") } - p := &lib.InitFSIDatasetParams{ - Dir: pwd, - Mkdir: o.Mkdir, - Format: o.Format, - Name: o.Name, - SourceBodyPath: o.SourceBodyPath, - UseDscache: o.UseDscache, + p := &lib.InitDatasetParams{ + TargetDir: o.TargetDir, + Format: o.Format, + Name: o.Name, + BodyPath: o.BodyPath, + UseDscache: o.UseDscache, } var refstr string if err = o.FSIMethods.InitDataset(p, &refstr); err != nil { diff --git a/cmd/test_runner_test.go b/cmd/test_runner_test.go index cc7390459..e752b76cc 100644 --- a/cmd/test_runner_test.go +++ b/cmd/test_runner_test.go @@ -222,6 +222,7 @@ func (run *TestRunner) ExecCommand(cmdText string) error { // ExecCommandWithStdin executes the given command string with the string as stdin content func (run *TestRunner) ExecCommandWithStdin(ctx context.Context, cmdText, stdinText string) error { setNoColor(true) + run.Streams.In = strings.NewReader(stdinText) cmd, shutdown := NewQriCommand(ctx, run.RepoPath, run.RepoRoot.TestCrypto, run.Streams) cmd.SetOutput(run.OutStream) run.CmdR = cmd diff --git a/fsi/init.go b/fsi/init.go index 68ca1c366..2c0d718d1 100644 --- a/fsi/init.go +++ b/fsi/init.go @@ -18,12 +18,11 @@ import ( // InitParams encapsulates parameters for fsi.InitDataset type InitParams struct { - Dir string - Name string - Format string - Mkdir string - SourceBodyPath string - UseDscache bool + TargetDir string + Name string + Format string + BodyPath string + UseDscache bool } func concatFunc(f1, f2 func()) func() { @@ -56,49 +55,31 @@ func (fsi *FSI) InitDataset(p InitParams) (ref dsref.Ref, err error) { if !dsref.IsValidName(p.Name) { return ref, dsref.ErrDescribeValidName } - if p.Dir == "" { + if p.TargetDir == "" { return ref, fmt.Errorf("directory is required to initialize a dataset") } - if fi, err := os.Stat(p.Dir); err != nil { - return ref, err - } else if !fi.IsDir() { - return ref, fmt.Errorf("invalid path to initialize. '%s' is not a directory", p.Dir) - } + baseDir, createDir := SplitDirByExist(p.TargetDir) - // Either use an existing directory, or create one at the given directory. - var targetPath string - if p.Mkdir == "" { - targetPath = p.Dir - } else { - targetPath = filepath.Join(p.Dir, p.Mkdir) - // Create the directory. It is not an error for the directory to already exist, as long - // as it is not already linked, which is checked below. - err := os.Mkdir(targetPath, os.ModePerm) - if err != nil { - if strings.Contains(err.Error(), "file exists") { - // Not an error if directory already exists - err = nil - } else { - return ref, err - } - } else { - // If directory was successfully created, add a step to the rollback in case future - // steps fail. - rollback = concatFunc( - func() { - log.Debugf("removing directory %q during rollback", targetPath) - if err := os.Remove(targetPath); err != nil { - log.Debugf("error while removing directory %q: %s", targetPath, err) - } - }, rollback) + if createDir != "" { + if err := os.MkdirAll(p.TargetDir, 0755); err != nil { + return ref, err } + rollback = concatFunc( + func() { + firstSegment := strings.Split(createDir, string(os.PathSeparator))[0] + cleanupPath := filepath.Join(baseDir, firstSegment) + log.Debugf("removing directory %q during rollback", cleanupPath) + if err := os.RemoveAll(cleanupPath); err != nil { + log.Debugf("error while removing directory %q: %s", cleanupPath, err) + } + }, rollback) } // Make sure we're not going to overwrite any files in the directory being initialized. - // Pass the sourceBodyPath, because it's okay if this file already exists, as long as its + // Pass the bodyPath, because it's okay if this file already exists, as long as its // being used to create the body. - if err = fsi.CanInitDatasetWorkDir(targetPath, p.SourceBodyPath); err != nil { + if err = fsi.CanInitDatasetWorkDir(p.TargetDir, p.BodyPath); err != nil { return ref, err } @@ -134,8 +115,8 @@ to create a working directory for an existing dataset` }, rollback) // Derive format from --body if provided. - if p.Format == "" && p.SourceBodyPath != "" { - ext := filepath.Ext(p.SourceBodyPath) + if p.Format == "" && p.BodyPath != "" { + ext := filepath.Ext(p.BodyPath) if len(ext) > 0 { p.Format = ext[1:] } @@ -151,14 +132,14 @@ to create a working directory for an existing dataset` // but I'd like to have one place that fires LinkCreated Events, maybe a private function // that both the exported CreateLink & Init can call vi := ref.VersionInfo() - vi.FSIPath = targetPath + vi.FSIPath = p.TargetDir if err := repo.PutVersionInfoShim(fsi.repo, &vi); err != nil { return ref, err } // Create the link file, containing the dataset reference. var undo func() - if _, undo, err = fsi.CreateLink(targetPath, ref); err != nil { + if _, undo, err = fsi.CreateLink(p.TargetDir, ref); err != nil { return ref, err } // If future steps fail, rollback the link creation. @@ -172,15 +153,15 @@ to create a working directory for an existing dataset` // Add body file. var bodySchema map[string]interface{} - if p.SourceBodyPath != "" { - initDs.BodyPath = p.SourceBodyPath + if p.BodyPath != "" { + initDs.BodyPath = p.BodyPath // Create structure by detecting it from the body. - file, err := os.Open(p.SourceBodyPath) + file, err := os.Open(p.BodyPath) if err != nil { return ref, err } defer file.Close() - // TODO(dlong): This should move into `dsio` package. + // TODO(dustmop): This should move into `dsio` package. entries, err := component.OpenEntryReader(file, p.Format) if err != nil { log.Errorf("opening entry reader: %s", err) @@ -215,7 +196,7 @@ to create a working directory for an existing dataset` for _, compName := range component.AllSubcomponentNames() { aComp := container.Base().GetSubcomponent(compName) if aComp != nil { - wroteFile, err := aComp.WriteTo(targetPath) + wroteFile, err := aComp.WriteTo(p.TargetDir) if err != nil { log.Errorf("writing component file %s: %s", compName, err) return ref, err @@ -238,9 +219,9 @@ to create a working directory for an existing dataset` } // CanInitDatasetWorkDir returns nil if the directory can init a dataset, or an error if not -func (fsi *FSI) CanInitDatasetWorkDir(dir, sourceBodyPath string) error { +func (fsi *FSI) CanInitDatasetWorkDir(dir, bodyPath string) error { // Get the body relative to the directory that we're initializing. - relBodyPath := sourceBodyPath + relBodyPath := bodyPath if strings.HasPrefix(relBodyPath, dir) { relBodyPath = strings.TrimPrefix(relBodyPath, dir) // Removing the directory may leave a leading slash, if the dirname did not end in a slash. @@ -253,8 +234,8 @@ func (fsi *FSI) CanInitDatasetWorkDir(dir, sourceBodyPath string) error { if linkfile.ExistsInDir(dir) { return fmt.Errorf("working directory is already linked, .qri-ref exists") } - // Check if other component files exist. If sourceBodyPath is provided, it's not an error - // if its filename exists. + // Check if other component files exist. If bodyPath is provided, it's not an error if its + // filename exists. if _, err := os.Stat(filepath.Join(dir, "meta.json")); !os.IsNotExist(err) { return fmt.Errorf("cannot initialize new dataset, meta.json exists") } @@ -274,3 +255,22 @@ func (fsi *FSI) CanInitDatasetWorkDir(dir, sourceBodyPath string) error { return nil } + +// SplitDirByExist splits a path into the part that exists, and the part that is missing +func SplitDirByExist(fullpath string) (string, string) { + segments := strings.Split(fullpath, string(os.PathSeparator)) + // Iterate path segments starting from root, check if each exists + for i := range segments { + if i < 1 { + continue + } + // Build path such that it includes the current segment being iterated + path := "/" + filepath.Join(segments[:i+1]...) + if _, err := os.Stat(path); os.IsNotExist(err) { + // If one is found to not exist, split on the current segment + return "/" + filepath.Join(segments[:i]...), filepath.Join(segments[i:]...) + } + } + // Entire path exists + return fullpath, "" +} diff --git a/fsi/init_test.go b/fsi/init_test.go index 329eb2d40..4215b81e4 100644 --- a/fsi/init_test.go +++ b/fsi/init_test.go @@ -1,6 +1,9 @@ package fsi import ( + "os" + "path/filepath" + "strings" "testing" ) @@ -11,11 +14,64 @@ func TestInitDataset(t *testing.T) { fsi := NewFSI(paths.testRepo, nil) _, err := fsi.InitDataset(InitParams{ - Name: "test_ds", - Dir: paths.firstDir, - Format: "csv", + Name: "test_ds", + TargetDir: paths.firstDir, + Format: "csv", }) if err != nil { t.Fatalf(err.Error()) } } + +func TestSplitDirByExist(t *testing.T) { + tmps := NewTmpPaths() + defer tmps.Close() + + // Create one directory + if err := os.Mkdir(filepath.Join(tmps.firstDir, "create"), 0755); err != nil { + t.Fatal(err) + } + // Split a path with two components after the existing directory + subjectDir := filepath.Join(tmps.firstDir, "create/this/path") + foundDir, missingDir := SplitDirByExist(subjectDir) + + if !strings.HasSuffix(foundDir, "/create") { + t.Errorf("expected foundDir to have suffix \"/create\", got: %q", foundDir) + } + expectDir := "this/path" + if missingDir != expectDir { + t.Errorf("expected: %q, got: %q", expectDir, missingDir) + } + + // Create another directory + if err := os.Mkdir(filepath.Join(tmps.firstDir, "create/this"), 0755); err != nil { + t.Fatal(err) + } + // Split again + subjectDir = filepath.Join(tmps.firstDir, "create/this/path") + foundDir, missingDir = SplitDirByExist(subjectDir) + + if !strings.HasSuffix(foundDir, "/create/this") { + t.Errorf("expected foundDir to have suffix \"/create/this\", got: %q", foundDir) + } + expectDir = "path" + if missingDir != expectDir { + t.Errorf("expected: %q, got: %q", expectDir, missingDir) + } + + // Create last directory + if err := os.Mkdir(filepath.Join(tmps.firstDir, "create/this/path"), 0755); err != nil { + t.Fatal(err) + } + // Split again + subjectDir = filepath.Join(tmps.firstDir, "create/this/path") + foundDir, missingDir = SplitDirByExist(subjectDir) + + if !strings.HasSuffix(foundDir, "/create/this/path") { + t.Errorf("expected foundDir to have suffix \"/create/this/path\", got: %q", foundDir) + } + expectDir = "" + if missingDir != expectDir { + t.Errorf("expected: %q, got: %q", expectDir, missingDir) + } +} diff --git a/fsi/status_test.go b/fsi/status_test.go index 617b7e9c6..ec84dfb5a 100644 --- a/fsi/status_test.go +++ b/fsi/status_test.go @@ -39,9 +39,9 @@ func TestStatusValid(t *testing.T) { fsi := NewFSI(paths.testRepo, nil) _, err := fsi.InitDataset(InitParams{ - Name: "test_ds", - Dir: paths.firstDir, - Format: "csv", + Name: "test_ds", + TargetDir: paths.firstDir, + Format: "csv", }) if err != nil { t.Fatalf(err.Error()) @@ -87,9 +87,9 @@ func TestStatusInvalidDataset(t *testing.T) { fsi := NewFSI(paths.testRepo, nil) _, err := fsi.InitDataset(InitParams{ - Name: "test_ds", - Dir: paths.firstDir, - Format: "csv", + Name: "test_ds", + TargetDir: paths.firstDir, + Format: "csv", }) if err != nil { t.Fatalf(err.Error()) @@ -115,9 +115,9 @@ func TestStatusInvalidMeta(t *testing.T) { fsi := NewFSI(paths.testRepo, nil) _, err := fsi.InitDataset(InitParams{ - Name: "test_ds", - Dir: paths.firstDir, - Format: "csv", + Name: "test_ds", + TargetDir: paths.firstDir, + Format: "csv", }) if err != nil { t.Fatalf(err.Error()) @@ -141,9 +141,9 @@ func TestStatusNotFound(t *testing.T) { fsi := NewFSI(paths.testRepo, nil) _, err := fsi.InitDataset(InitParams{ - Name: "test_ds", - Dir: paths.firstDir, - Format: "csv", + Name: "test_ds", + TargetDir: paths.firstDir, + Format: "csv", }) if err != nil { t.Fatalf(err.Error()) diff --git a/lib/datasets_test.go b/lib/datasets_test.go index a6c23d032..e963887ef 100644 --- a/lib/datasets_test.go +++ b/lib/datasets_test.go @@ -776,10 +776,10 @@ func TestRenameNoHistory(t *testing.T) { defer tr.Delete() workDir := tr.CreateAndChdirToWorkDir("remove_no_history") - initP := &InitFSIDatasetParams{ - Dir: workDir, - Name: "remove_no_history", - Format: "csv", + initP := &InitDatasetParams{ + TargetDir: workDir, + Name: "remove_no_history", + Format: "csv", } var refstr string if err := NewFSIMethods(tr.Instance).InitDataset(initP, &refstr); err != nil { @@ -838,11 +838,10 @@ func TestDatasetRequestsRemove(t *testing.T) { defer os.RemoveAll(datasetsDir) // initialize an example no-history dataset - initp := &InitFSIDatasetParams{ - Name: "no_history", - Dir: datasetsDir, - Format: "csv", - Mkdir: "no_history", + initp := &InitDatasetParams{ + Name: "no_history", + TargetDir: filepath.Join(datasetsDir, "no_history"), + Format: "csv", } var noHistoryName string if err := fsim.InitDataset(initp, &noHistoryName); err != nil { @@ -1121,11 +1120,10 @@ func TestDatasetRequestsValidateFSI(t *testing.T) { defer tr.Delete() workDir := tr.CreateAndChdirToWorkDir("remove_no_history") - initP := &InitFSIDatasetParams{ - Name: "validate_test", - Dir: workDir, - Format: "csv", - Mkdir: "validate_test", + initP := &InitDatasetParams{ + Name: "validate_test", + TargetDir: filepath.Join(workDir, "validate_test"), + Format: "csv", } var refstr string if err := NewFSIMethods(tr.Instance).InitDataset(initP, &refstr); err != nil { diff --git a/lib/fsi.go b/lib/fsi.go index 451b35eef..7929ba61b 100644 --- a/lib/fsi.go +++ b/lib/fsi.go @@ -340,12 +340,19 @@ func (m *FSIMethods) Restore(p *RestoreParams, out *string) (err error) { return nil } -// InitFSIDatasetParams proxies parameters to initialization -type InitFSIDatasetParams = fsi.InitParams +// InitDatasetParams proxies parameters to initialization +type InitDatasetParams = fsi.InitParams -// InitDataset creates a new dataset and FSI link -func (m *FSIMethods) InitDataset(p *InitFSIDatasetParams, refstr *string) (err error) { - if err = qfs.AbsPath(&p.SourceBodyPath); err != nil { +// InitDataset creates a new dataset in a working directory +func (m *FSIMethods) InitDataset(p *InitDatasetParams, refstr *string) (err error) { + if err = qfs.AbsPath(&p.BodyPath); err != nil { + return err + } + + if p.TargetDir == "" { + p.TargetDir = "." + } + if err = qfs.AbsPath(&p.TargetDir); err != nil { return err } @@ -364,10 +371,10 @@ func (m *FSIMethods) InitDataset(p *InitFSIDatasetParams, refstr *string) (err e } // CanInitDatasetWorkDir returns nil if the directory can init a dataset, or an error if not -func (m *FSIMethods) CanInitDatasetWorkDir(p *InitFSIDatasetParams, ok *bool) error { - dir := p.Dir - sourceBodyPath := p.SourceBodyPath - return m.inst.fsi.CanInitDatasetWorkDir(dir, sourceBodyPath) +func (m *FSIMethods) CanInitDatasetWorkDir(p *InitDatasetParams, ok *bool) error { + targetPath := p.TargetDir + bodyPath := p.BodyPath + return m.inst.fsi.CanInitDatasetWorkDir(targetPath, bodyPath) } // EnsureParams holds values for EnsureRef call diff --git a/lib/fsi_test.go b/lib/fsi_test.go index 9d4786299..0eb8016dd 100644 --- a/lib/fsi_test.go +++ b/lib/fsi_test.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "sort" "testing" cmp "github.com/google/go-cmp/cmp" @@ -42,11 +43,10 @@ func TestFSIMethodsWrite(t *testing.T) { defer os.RemoveAll(datasetsDir) // initialize an example no-history dataset - initp := &InitFSIDatasetParams{ - Name: "no_history", - Dir: datasetsDir, - Format: "csv", - Mkdir: "no_history", + initp := &InitDatasetParams{ + Name: "no_history", + TargetDir: filepath.Join(datasetsDir, "no_history"), + Format: "csv", } var noHistoryName string if err := methods.InitDataset(initp, &noHistoryName); err != nil { @@ -281,3 +281,118 @@ func TestDscacheInit(t *testing.T) { t.Errorf("result mismatch (-want +got):%s\n", diff) } } + +// Test that `init` with no directory will use the current one +func TestInitWithCurrentDir(t *testing.T) { + run := newTestRunner(t) + defer run.Delete() + + initDir := filepath.Join(run.TmpDir, "path") + + // Create a directory that will become the working directory + if err := os.Mkdir(initDir, 0755); err != nil { + t.Fatal(err) + } + // Change to it + if err := os.Chdir(initDir); err != nil { + t.Fatal(err) + } + + // Don't pass the working directory path, `init` will use the current directory + err := run.InitWithParams( + &InitDatasetParams{ + Name: "new_ds", + Format: "csv", + }, + ) + if err != nil { + t.Fatal(err) + } + + // Verify the directory contains the files that we expect + dirContents := listDirectory(initDir) + expectContents := []string{".qri-ref", "body.csv", "meta.json", "structure.json"} + if diff := cmp.Diff(expectContents, dirContents); diff != "" { + t.Errorf("directory contents did not match (-want +got):\n%s", diff) + } +} + +// Test that `init` with an explicit directory will create it +func TestInitWithExplicitDir(t *testing.T) { + run := newTestRunner(t) + defer run.Delete() + + initDir := filepath.Join(run.TmpDir, "path/to/dataset") + + // Pass the path for the directory, though it doesn't exist yet, `init` will make it + err := run.InitWithParams( + &InitDatasetParams{ + Name: "new_ds", + TargetDir: initDir, + Format: "csv", + }, + ) + if err != nil { + t.Fatal(err) + } + + // Verify the directory contains the files that we expect + dirContents := listDirectory(initDir) + expectContents := []string{".qri-ref", "body.csv", "meta.json", "structure.json"} + if diff := cmp.Diff(expectContents, dirContents); diff != "" { + t.Errorf("directory contents did not match (-want +got):\n%s", diff) + } +} + +// Test that if `init` fails after requested directory has been created, that directory +// will be removed as part of rollback +func TestInitRollbackRemovesDirectory(t *testing.T) { + run := newTestRunner(t) + defer run.Delete() + + // Save a dataset with one version. + run.MustSaveFromBody(t, "movie_ds", "testdata/cities_2/body.csv") + // Change to the temp directory to create some directory + run.ChdirToRoot() + + // Create one directory, will run `qri init` targeting a non-existent subdirectory + if err := os.Mkdir("path", 0644); err != nil { + t.Fatal(err) + } + + // Init should fail because the reference already exists in logbook + // TODO(dustmop): Change this to some "fake" resolver, which holds references in memory, + // and use dependency injection for the test. Remove the need to add a reference to logbook. + err := run.InitWithParams( + &InitDatasetParams{ + Name: "movie_ds", + TargetDir: filepath.Join(run.TmpDir, "path/to/dataset"), + Format: "csv", + }, + ) + if err == nil { + t.Fatal("expected error from init, did not get one") + } + + // Directory "./path/to/" does not exist + if _, err = os.Stat("path/to/"); err == nil { + t.Error("directory \"./path/to\" should have been removed by rollback") + } + // Directory "./path/" should still exist + if _, err = os.Stat("path"); os.IsNotExist(err) { + t.Error("directory \"./path\" should *NOT* have been removed by rollback") + } +} + +func listDirectory(path string) []string { + contents := []string{} + finfos, err := ioutil.ReadDir(path) + if err != nil { + return nil + } + for _, fi := range finfos { + contents = append(contents, fi.Name()) + } + sort.Strings(contents) + return contents +} diff --git a/lib/test_runner_test.go b/lib/test_runner_test.go index da8617606..546635c45 100644 --- a/lib/test_runner_test.go +++ b/lib/test_runner_test.go @@ -209,14 +209,20 @@ func (tr *testRunner) Init(refstr, format string) error { } m := NewFSIMethods(tr.Instance) out := "" - p := InitFSIDatasetParams{ - Name: ref.Name, - Dir: tr.WorkDir, - Format: format, + p := InitDatasetParams{ + Name: ref.Name, + TargetDir: tr.WorkDir, + Format: format, } return m.InitDataset(&p, &out) } +func (tr *testRunner) InitWithParams(p *InitDatasetParams) error { + m := NewFSIMethods(tr.Instance) + out := "" + return m.InitDataset(p, &out) +} + func (tr *testRunner) Checkout(refstr, dir string) error { m := NewFSIMethods(tr.Instance) out := ""