fix(load): Fix spec test to exercise LoadDataset

dsref/spec/load.go

@@ -13,74 +13,91 @@ import (
 
 // PutDatasetFunc adds a dataset to a system that stores datasets
 // PutDatasetFunc is required to run the LoaderSpec test. When called the
-// Loader should retain the dataset for later loading by the spec test
-type PutDatasetFunc func(ds *dataset.Dataset) (path string, err error)
+// Loader should retain the dataset for later loading by the spec test, and
+// return a full reference to the saved version
+type PutDatasetFunc func(ds *dataset.Dataset) (ref *dsref.Ref, err error)
 
 // AssertLoaderSpec confirms the expected behaviour of a dsref.Loader
 // Interface implementation. In addition to this test passing, implementations
 // MUST be nil-callable. Please add a nil-callable test for each implementation
 //
-// TODO(b5) - loader spec is intentionally a little vague at the moment. I'm not
-// sure the interface belongs in this package, and this test isn't working
-// network sources. This test serves to confirm basic requirements of a local
-// loader function for the moment
+// TODO(b5) - this test isn't working network sources. At the moment it confirms
+// only basic requirements of a local loader function
 func AssertLoaderSpec(t *testing.T, r dsref.Loader, putFunc PutDatasetFunc) {
-	t.Helper()
-
-	var (
-		ctx      = context.Background()
-		username = "example_user"
-		name     = "this_is_an_example"
-		path     = ""
-	)
+	ctx := context.Background()
 
 	ds, err := GenerateExampleDataset(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	path, err = putFunc(ds)
+	savedRef, err := putFunc(ds)
 	if err != nil {
 		t.Fatalf("putting dataset: %s", err)
 	}
 
-	_, err = r.LoadDataset(ctx, "")
-	if err == nil {
-		t.Errorf("expected loading without a reference Path value to fail, got nil")
-	}
+	// update our expected dataset with values from the added ref
+	ds.Peername = savedRef.Username
+	ds.Name = savedRef.Name
+	ds.Path = savedRef.Path
+	// TODO(b5): loading should require initID to be set
+	// ds.ID = savedRef.InitID
 
-	ref := dsref.Ref{
-		Username: username,
-		Name:     name,
-		Path:     path,
-	}
-	got, err := r.LoadDataset(ctx, ref.String())
-	if err != nil {
-		t.Fatal(err)
-	}
+	t.Run("empty_ref", func(t *testing.T) {
+		if _, err = r.LoadDataset(ctx, ""); err == nil {
+			t.Errorf("expected loading an empty string to fail, did not get an error")
+		}
+	})
 
-	if got.BodyFile() == nil {
-		t.Errorf("expected body file to be open & ready to read")
-	}
+	t.Run("full_reference_provided", func(t *testing.T) {
+		got, err := r.LoadDataset(ctx, savedRef.String())
+		if err != nil {
+			t.Fatal(err)
+		}
 
-	if username != got.Peername {
-		t.Errorf("load Dataset didn't set dataset.Peername field to given reference. want: %q got: %q", username, got.Peername)
-	}
-	if name != got.Name {
-		t.Errorf("load Dataset didn't set dataset.Name field to given reference. want: %q got: %q", name, got.Name)
-	}
+		if got.BodyFile() == nil {
+			t.Errorf("expected body file to be open & ready to read")
+		}
 
-	ds.Peername = username
-	ds.Name = name
-	ds.Path = path
-	if diff := cmp.Diff(ds, got, cmpopts.IgnoreUnexported(dataset.Dataset{}, dataset.Meta{})); diff != "" {
-		t.Errorf("result mismatch (-want +got):\n%s", diff)
-	}
+		if savedRef.Username != got.Peername {
+			t.Errorf("load Dataset didn't set dataset.Peername field to given reference. want: %q got: %q", savedRef.Username, got.Peername)
+		}
+		if savedRef.Name != got.Name {
+			t.Errorf("load Dataset didn't set dataset.Name field to given reference. want: %q got: %q", savedRef.Name, got.Name)
+		}
+
+		if diff := cmp.Diff(ds, got, cmpopts.IgnoreUnexported(dataset.Dataset{}, dataset.Meta{})); diff != "" {
+			t.Errorf("result mismatch (-want +got):\n%s", diff)
+		}
+	})
+
+	t.Run("alias_provided", func(t *testing.T) {
+		got, err := r.LoadDataset(ctx, savedRef.Alias())
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if got.BodyFile() == nil {
+			t.Errorf("expected body file to be open & ready to read")
+		}
+
+		if savedRef.Username != got.Peername {
+			t.Errorf("load Dataset didn't set dataset.Peername field to given reference. want: %q got: %q", savedRef.Username, got.Peername)
+		}
+		if savedRef.Name != got.Name {
+			t.Errorf("load Dataset didn't set dataset.Name field to given reference. want: %q got: %q", savedRef.Name, got.Name)
+		}
+
+		if diff := cmp.Diff(ds, got, cmpopts.IgnoreUnexported(dataset.Dataset{}, dataset.Meta{})); diff != "" {
+			t.Errorf("result mismatch (-want +got):\n%s", diff)
+		}
+	})
 }
 
 // GenerateExampleDataset creates an example dataset document
 func GenerateExampleDataset(ctx context.Context) (*dataset.Dataset, error) {
 	ds := &dataset.Dataset{
+		Name: "example_loader_spec_test",
 		Commit: &dataset.Commit{
 			Title: "initial commit",
 		},

lib/load.go

@@ -75,6 +75,9 @@ func (d *datasetLoader) loadLocalDataset(ctx context.Context, ref dsref.Ref) (*d
 	// Set transient info on the returned dataset
 	ds.Name = ref.Name
 	ds.Peername = ref.Username
+	// TODO(dustmop): When dscache / dscollect is in use, enable this since resolved
+	// references should always have it set
+	// ds.ID = ref.InitID
 
 	if err = base.OpenDataset(ctx, d.inst.repo.Filesystem(), ds); err != nil {
 		log.Debugf("Get dataset, base.OpenDataset failed, error: %s", err)

lib/load_test.go

@@ -5,13 +5,12 @@ import (
 
 	"github.com/qri-io/dataset"
 	"github.com/qri-io/qri/base/dsfs"
+	"github.com/qri-io/qri/dsref"
 	dsrefspec "github.com/qri-io/qri/dsref/spec"
 	"github.com/qri-io/qri/event"
 )
 
 func TestLoadDataset(t *testing.T) {
-	t.Skip("TODO(dustmop): Change in LoadDataset semantics breaks this test, figure out why")
-
 	tr := newTestRunner(t)
 	defer tr.Delete()
 
@@ -27,8 +26,19 @@ func TestLoadDataset(t *testing.T) {
 	}
 
 	loader = &datasetLoader{inst: tr.Instance}
-	dsrefspec.AssertLoaderSpec(t, loader, func(ds *dataset.Dataset) (string, error) {
-		return dsfs.CreateDataset(
+	dsrefspec.AssertLoaderSpec(t, loader, func(ds *dataset.Dataset) (*dsref.Ref, error) {
+		// Allocate an initID for this dataset
+		initID, err := tr.Instance.logbook.WriteDatasetInit(tr.Ctx, ds.Name)
+		if err != nil {
+			return nil, err
+		}
+		// Create the dataset in the provided storage
+		ref := &dsref.Ref{
+			InitID:   initID,
+			Username: tr.Instance.repo.Profiles().Owner().Peername,
+			Name:     ds.Name,
+		}
+		path, err := dsfs.CreateDataset(
 			tr.Ctx,
 			fs,
 			fs.DefaultWriteFS(),
@@ -38,5 +48,15 @@ func TestLoadDataset(t *testing.T) {
 			tr.Instance.repo.Profiles().Owner().PrivKey,
 			dsfs.SaveSwitches{},
 		)
+		if err != nil {
+			return nil, err
+		}
+		// Save the reference that the loader will use to laod
+		ref.Path = path
+		ds.Path = path
+		if err = tr.Instance.logbook.WriteVersionSave(tr.Ctx, initID, ds, nil); err != nil {
+			return nil, err
+		}
+		return ref, nil
 	})
 }