Add rootdata-specific tests

Author: sdboyer

lock.go

@@ -212,7 +212,7 @@ func (sl safeLock) Projects() []LockedProject {
 // while the solver is in-flight.
 //
 // This is achieved by copying the lock's data into a new safeLock.
-func prepLock(l Lock) Lock {
+func prepLock(l Lock) safeLock {
 	pl := l.Projects()
 
 	rl := safeLock{

manifest.go

@@ -139,7 +139,7 @@ func (m simpleRootManifest) dup() simpleRootManifest {
 // ProjectProperties.
 //
 // This is achieved by copying the manifest's data into a new SimpleManifest.
-func prepManifest(m Manifest) Manifest {
+func prepManifest(m Manifest) SimpleManifest {
 	if m == nil {
 		return SimpleManifest{}
 	}

rootdata.go

@@ -32,13 +32,13 @@ type rootdata struct {
 	// A map of the project names listed in the root's lock.
 	rlm map[ProjectRoot]LockedProject
 
-	// A defensively-copied instance of the root manifest.
-	rm Manifest
+	// A defensively copied instance of the root manifest.
+	rm SimpleManifest
 
-	// A defensively-copied instance of the root lock.
-	rl Lock
+	// A defensively copied instance of the root lock.
+	rl safeLock
 
-	// A defensively-copied instance of params.RootPackageTree
+	// A defensively copied instance of params.RootPackageTree
 	rpt PackageTree
 }
 
@@ -55,10 +55,8 @@ func (rd rootdata) externalImportList() []string {
 
 	// If there are any requires, slide them into the reach list, as well.
 	if len(rd.req) > 0 {
-		// Make a map of both imported and required pkgs to skip, to avoid
-		// duplication. Technically, a slice would probably be faster (given
-		// small size and bounds check elimination), but this is a one-time op,
-		// so it doesn't matter.
+		// Make a map of imports that are both in the import path list and the
+		// required list to avoid duplication.
 		skip := make(map[string]bool, len(rd.req))
 		for _, r := range reach {
 			if rd.req[r] {
@@ -143,10 +141,10 @@ func (rd rootdata) combineConstraints() []workingConstraint {
 
 // needVersionListFor indicates whether we need a version list for a given
 // project root, based solely on general solver inputs (no constraint checking
-// required). This will be true if:
+// required). This will be true if any of the following conditions hold:
 //
 //  - ChangeAll is on
-//  - The project is not in the lock at all
+//  - The project is not in the lock
 //  - The project is in the lock, but is also in the list of projects to change
 func (rd rootdata) needVersionsFor(pr ProjectRoot) bool {
 	if rd.chngall {

rootdata_test.go

@@ -0,0 +1,214 @@
+package gps
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestRootdataExternalImports(t *testing.T) {
+	fix := basicFixtures["shared dependency with overlapping constraints"]
+
+	params := SolveParameters{
+		RootDir:         string(fix.ds[0].n),
+		RootPackageTree: fix.rootTree(),
+		Manifest:        fix.rootmanifest(),
+	}
+
+	is, err := Prepare(params, newdepspecSM(fix.ds, nil))
+	if err != nil {
+		t.Errorf("Unexpected error while prepping solver: %s", err)
+		t.FailNow()
+	}
+	rd := is.(*solver).rd
+
+	want := []string{"a", "b"}
+	got := rd.externalImportList()
+	if !reflect.DeepEqual(want, got) {
+		t.Errorf("Unexpected return from rootdata.externalImportList:\n\t(GOT): %s\n\t(WNT): %s", got, want)
+	}
+
+	// Add a require
+	rd.req["c"] = true
+
+	want = []string{"a", "b", "c"}
+	got = rd.externalImportList()
+	if !reflect.DeepEqual(want, got) {
+		t.Errorf("Unexpected return from rootdata.externalImportList:\n\t(GOT): %s\n\t(WNT): %s", got, want)
+	}
+
+	// Add same path as import
+	poe := rd.rpt.Packages["root"]
+	poe.P.Imports = []string{"a", "b", "c"}
+	rd.rpt.Packages["root"] = poe
+
+	// should still be the same
+	got = rd.externalImportList()
+	if !reflect.DeepEqual(want, got) {
+		t.Errorf("Unexpected return from rootdata.externalImportList:\n\t(GOT): %s\n\t(WNT): %s", got, want)
+	}
+
+	// Add an ignore, but not on the required path (Prepare makes that
+	// combination impossible)
+
+	rd.ig["b"] = true
+	want = []string{"a", "c"}
+	got = rd.externalImportList()
+	if !reflect.DeepEqual(want, got) {
+		t.Errorf("Unexpected return from rootdata.externalImportList:\n\t(GOT): %s\n\t(WNT): %s", got, want)
+	}
+}
+
+func TestGetApplicableConstraints(t *testing.T) {
+	fix := basicFixtures["shared dependency with overlapping constraints"]
+
+	params := SolveParameters{
+		RootDir:         string(fix.ds[0].n),
+		RootPackageTree: fix.rootTree(),
+		Manifest:        fix.rootmanifest(),
+	}
+
+	is, err := Prepare(params, newdepspecSM(fix.ds, nil))
+	if err != nil {
+		t.Errorf("Unexpected error while prepping solver: %s", err)
+		t.FailNow()
+	}
+	rd := is.(*solver).rd
+
+	table := []struct {
+		name   string
+		mut    func()
+		result []workingConstraint
+	}{
+		{
+			name: "base case, two constraints",
+			mut:  func() {},
+			result: []workingConstraint{
+				{
+					Ident:      mkPI("a"),
+					Constraint: mkSVC("1.0.0"),
+				},
+				{
+					Ident:      mkPI("b"),
+					Constraint: mkSVC("1.0.0"),
+				},
+			},
+		},
+		{
+			name: "with unconstrained require",
+			mut: func() {
+				// No constraint means it doesn't show up
+				rd.req["c"] = true
+			},
+			result: []workingConstraint{
+				{
+					Ident:      mkPI("a"),
+					Constraint: mkSVC("1.0.0"),
+				},
+				{
+					Ident:      mkPI("b"),
+					Constraint: mkSVC("1.0.0"),
+				},
+			},
+		},
+		{
+			name: "with unconstrained import",
+			mut: func() {
+				// Again, no constraint means it doesn't show up
+				poe := rd.rpt.Packages["root"]
+				poe.P.Imports = []string{"a", "b", "d"}
+				rd.rpt.Packages["root"] = poe
+			},
+			result: []workingConstraint{
+				{
+					Ident:      mkPI("a"),
+					Constraint: mkSVC("1.0.0"),
+				},
+				{
+					Ident:      mkPI("b"),
+					Constraint: mkSVC("1.0.0"),
+				},
+			},
+		},
+		{
+			name: "constraint on required",
+			mut: func() {
+				rd.rm.Deps["c"] = ProjectProperties{
+					Constraint: NewBranch("foo"),
+				}
+			},
+			result: []workingConstraint{
+				{
+					Ident:      mkPI("a"),
+					Constraint: mkSVC("1.0.0"),
+				},
+				{
+					Ident:      mkPI("b"),
+					Constraint: mkSVC("1.0.0"),
+				},
+				{
+					Ident:      mkPI("c"),
+					Constraint: NewBranch("foo"),
+				},
+			},
+		},
+		{
+			name: "override on imported",
+			mut: func() {
+				rd.ovr["d"] = ProjectProperties{
+					Constraint: NewBranch("bar"),
+				}
+			},
+			result: []workingConstraint{
+				{
+					Ident:      mkPI("a"),
+					Constraint: mkSVC("1.0.0"),
+				},
+				{
+					Ident:      mkPI("b"),
+					Constraint: mkSVC("1.0.0"),
+				},
+				{
+					Ident:      mkPI("c"),
+					Constraint: NewBranch("foo"),
+				},
+				{
+					Ident:           mkPI("d"),
+					Constraint:      NewBranch("bar"),
+					overrConstraint: true,
+				},
+			},
+		},
+		{
+			// It is certainly the simplest and most rule-abiding solution to
+			// drop the constraint in this case, but is there a chance it would
+			// violate the principle of least surprise?
+			name: "ignore imported and overridden pkg",
+			mut: func() {
+				rd.ig["d"] = true
+			},
+			result: []workingConstraint{
+				{
+					Ident:      mkPI("a"),
+					Constraint: mkSVC("1.0.0"),
+				},
+				{
+					Ident:      mkPI("b"),
+					Constraint: mkSVC("1.0.0"),
+				},
+				{
+					Ident:      mkPI("c"),
+					Constraint: NewBranch("foo"),
+				},
+			},
+		},
+	}
+
+	for _, fix := range table {
+		fix.mut()
+
+		got := rd.getApplicableConstraints()
+		if !reflect.DeepEqual(fix.result, got) {
+			t.Errorf("(fix: %q) unexpected applicable constraint set:\n\t(GOT): %+v\n\t(WNT): %+v", fix.name, got, fix.result)
+		}
+	}
+}