From 89669d915950963a82539afe91a47b9b6e5141d3 Mon Sep 17 00:00:00 2001 From: sam boyer Date: Sun, 8 Jul 2018 21:20:28 -0400 Subject: [PATCH] gps/verify: Add tests for LockSatisfaction Also better prepare it for public consumption by exporting its members, renaming some methods and improving docs. --- cmd/dep/ensure.go | 10 +- cmd/dep/status.go | 2 +- gps/verify/helper_types_test.go | 78 ++++++++++ gps/verify/locksat.go | 89 +++++------ gps/verify/locksat_test.go | 266 ++++++++++++++++++++++++++++++++ 5 files changed, 391 insertions(+), 54 deletions(-) create mode 100644 gps/verify/helper_types_test.go create mode 100644 gps/verify/locksat_test.go diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index 011834a44f..1f6f6948d1 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -282,19 +282,19 @@ func (cmd *ensureCommand) runDefault(ctx *dep.Ctx, args []string, p *dep.Project lock := p.ChangedLock if lock != nil { lsat := verify.LockSatisfiesInputs(p.Lock, p.Manifest, params.RootPackageTree) - if !lsat.Passed() { + if !lsat.Satisfied() { if ctx.Verbose { ctx.Out.Println("Gopkg.lock is out of sync with Gopkg.toml and project code:") - for _, missing := range lsat.MissingImports() { + for _, missing := range lsat.MissingImports { ctx.Out.Printf("\t%s is missing from input-imports\n", missing) } - for _, excess := range lsat.ExcessImports() { + for _, excess := range lsat.ExcessImports { ctx.Out.Printf("\t%s is in input-imports, but isn't imported\n", excess) } - for pr, unmatched := range lsat.UnmatchedOverrides() { + for pr, unmatched := range lsat.UnmetOverrides { ctx.Out.Printf("\t%s is at %s, which is not allowed by override %s\n", pr, unmatched.V, unmatched.C) } - for pr, unmatched := range lsat.UnmatchedConstraints() { + for pr, unmatched := range lsat.UnmetConstraints { ctx.Out.Printf("\t%s is at %s, which is not allowed by constraint %s\n", pr, unmatched.V, unmatched.C) } ctx.Out.Println() diff --git a/cmd/dep/status.go b/cmd/dep/status.go index 77e7671f26..f87c78679e 100644 --- a/cmd/dep/status.go +++ b/cmd/dep/status.go @@ -925,7 +925,7 @@ func (cmd *statusCommand) runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Proje }) lsat := verify.LockSatisfiesInputs(p.Lock, p.Manifest, params.RootPackageTree) - if lsat.Passed() { + if lsat.Satisfied() { // If these are equal, we're guaranteed that the lock is a transitively // complete picture of all deps. That eliminates the need for at least // some checks. diff --git a/gps/verify/helper_types_test.go b/gps/verify/helper_types_test.go new file mode 100644 index 0000000000..b27cf7eb2e --- /dev/null +++ b/gps/verify/helper_types_test.go @@ -0,0 +1,78 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package verify + +import ( + "github.com/golang/dep/gps" + "github.com/golang/dep/gps/pkgtree" +) + +// mkPI creates a ProjectIdentifier with the ProjectRoot as the provided +// string, and the Source unset. +// +// Call normalize() on the returned value if you need the Source to be be +// equal to the ProjectRoot. +func mkPI(root string) gps.ProjectIdentifier { + return gps.ProjectIdentifier{ + ProjectRoot: gps.ProjectRoot(root), + } +} + +type safeLock struct { + p []gps.LockedProject + i []string +} + +func (sl safeLock) InputImports() []string { + return sl.i +} + +func (sl safeLock) Projects() []gps.LockedProject { + return sl.p +} + +// simpleRootManifest exists so that we have a safe value to swap into solver +// params when a nil Manifest is provided. +type simpleRootManifest struct { + c, ovr gps.ProjectConstraints + ig *pkgtree.IgnoredRuleset + req map[string]bool +} + +func (m simpleRootManifest) DependencyConstraints() gps.ProjectConstraints { + return m.c +} +func (m simpleRootManifest) Overrides() gps.ProjectConstraints { + return m.ovr +} +func (m simpleRootManifest) IgnoredPackages() *pkgtree.IgnoredRuleset { + return m.ig +} +func (m simpleRootManifest) RequiredPackages() map[string]bool { + return m.req +} + +func (m simpleRootManifest) dup() simpleRootManifest { + m2 := simpleRootManifest{ + c: make(gps.ProjectConstraints), + ovr: make(gps.ProjectConstraints), + ig: pkgtree.NewIgnoredRuleset(m.ig.ToSlice()), + req: make(map[string]bool), + } + + for k, v := range m.c { + m2.c[k] = v + } + + for k, v := range m.ovr { + m2.ovr[k] = v + } + + for k := range m.req { + m2.req[k] = true + } + + return m2 +} diff --git a/gps/verify/locksat.go b/gps/verify/locksat.go index 0d5a9a4a1f..f2a7d4f0f3 100644 --- a/gps/verify/locksat.go +++ b/gps/verify/locksat.go @@ -9,10 +9,26 @@ import ( // LockSatisfaction holds the compound result of LockSatisfiesInputs, allowing // the caller to inspect each of several orthogonal possible types of failure. +// +// The zero value assumes that there was no input lock, which necessarily means +// the inputs were not satisfied. This zero value means we err on the side of +// failure. type LockSatisfaction struct { - nolock bool - missingPkgs, excessPkgs []string - badovr, badconstraint map[gps.ProjectRoot]ConstraintMismatch + // If LockExisted is false, it indicates that a nil gps.Lock was passed to + // LockSatisfiesInputs(). + LockExisted bool + // MissingImports is the set of import paths that were present in the + // inputs but missing in the Lock. + MissingImports []string + // ExcessImports is the set of import paths that were present in the Lock + // but absent from the inputs. + ExcessImports []string + // UnmatchedConstraints reports any normal, non-override constraint rules that + // were not satisfied by the corresponding LockedProject in the Lock. + UnmetConstraints map[gps.ProjectRoot]ConstraintMismatch + // UnmatchedOverrides reports any override rules that were not satisfied by the + // corresponding LockedProject in the Lock. + UnmetOverrides map[gps.ProjectRoot]ConstraintMismatch } // ConstraintMismatch is a two-tuple of a gps.Version, and a gps.Constraint that @@ -30,9 +46,15 @@ type ConstraintMismatch struct { // compute package imports that may have been removed. Figuring out that // negative space would require exploring the entire graph to ensure there are // no in-edges for particular imports. -func LockSatisfiesInputs(l gps.Lock, m gps.RootManifest, rpt pkgtree.PackageTree) LockSatisfaction { +func LockSatisfiesInputs(l gps.Lock, m gps.RootManifest, ptree pkgtree.PackageTree) LockSatisfaction { if l == nil { - return LockSatisfaction{nolock: true} + return LockSatisfaction{} + } + + lsat := LockSatisfaction{ + LockExisted: true, + UnmetOverrides: make(map[gps.ProjectRoot]ConstraintMismatch), + UnmetConstraints: make(map[gps.ProjectRoot]ConstraintMismatch), } var ig *pkgtree.IgnoredRuleset @@ -42,7 +64,7 @@ func LockSatisfiesInputs(l gps.Lock, m gps.RootManifest, rpt pkgtree.PackageTree req = m.RequiredPackages() } - rm, _ := rpt.ToReachMap(true, true, false, ig) + rm, _ := ptree.ToReachMap(true, true, false, ig) reach := rm.FlattenFn(paths.IsStandardImportPath) inlock := make(map[string]bool, len(l.InputImports())) @@ -68,11 +90,6 @@ func LockSatisfiesInputs(l gps.Lock, m gps.RootManifest, rpt pkgtree.PackageTree inlock[imp] = true } - lsat := LockSatisfaction{ - badovr: make(map[gps.ProjectRoot]ConstraintMismatch), - badconstraint: make(map[gps.ProjectRoot]ConstraintMismatch), - } - for ip := range ininputs { if !inlock[ip] { pkgDiff[ip] = missingFromLock @@ -96,9 +113,9 @@ func LockSatisfiesInputs(l gps.Lock, m gps.RootManifest, rpt pkgtree.PackageTree for ip, typ := range pkgDiff { if typ == missingFromLock { - lsat.missingPkgs = append(lsat.missingPkgs, ip) + lsat.MissingImports = append(lsat.MissingImports, ip) } else { - lsat.excessPkgs = append(lsat.excessPkgs, ip) + lsat.ExcessImports = append(lsat.ExcessImports, ip) } } @@ -110,7 +127,7 @@ func LockSatisfiesInputs(l gps.Lock, m gps.RootManifest, rpt pkgtree.PackageTree if pp, has := ovr[pr]; has { if !pp.Constraint.Matches(lp.Version()) { - lsat.badovr[pr] = ConstraintMismatch{ + lsat.UnmetOverrides[pr] = ConstraintMismatch{ C: pp.Constraint, V: lp.Version(), } @@ -121,7 +138,7 @@ func LockSatisfiesInputs(l gps.Lock, m gps.RootManifest, rpt pkgtree.PackageTree } if pp, has := constraints[pr]; has && eff[string(pr)] && !pp.Constraint.Matches(lp.Version()) { - lsat.badconstraint[pr] = ConstraintMismatch{ + lsat.UnmetConstraints[pr] = ConstraintMismatch{ C: pp.Constraint, V: lp.Version(), } @@ -131,57 +148,33 @@ func LockSatisfiesInputs(l gps.Lock, m gps.RootManifest, rpt pkgtree.PackageTree return lsat } -// Passed is a shortcut method that indicates whether there were any ways in -// which the Lock did not satisfy the inputs. It will return true only if no -// problems were found. -func (ls LockSatisfaction) Passed() bool { - if ls.nolock { +// Satisfied is a shortcut method that indicates whether there were any ways in +// which the Lock did not satisfy the inputs. It will return true only if the +// Lock was satisfactory in all respects vis-a-vis the inputs. +func (ls LockSatisfaction) Satisfied() bool { + if !ls.LockExisted { return false } - if len(ls.missingPkgs) > 0 { + if len(ls.MissingImports) > 0 { return false } - if len(ls.excessPkgs) > 0 { + if len(ls.ExcessImports) > 0 { return false } - if len(ls.badovr) > 0 { + if len(ls.UnmetOverrides) > 0 { return false } - if len(ls.badconstraint) > 0 { + if len(ls.UnmetConstraints) > 0 { return false } return true } -// MissingImports reports the set of import paths that were present in the -// inputs but missing in the Lock. -func (ls LockSatisfaction) MissingImports() []string { - return ls.missingPkgs -} - -// ExcessImports reports the set of import paths that were present in the Lock -// but absent from the inputs. -func (ls LockSatisfaction) ExcessImports() []string { - return ls.excessPkgs -} - -// UnmatchedOverrides reports any override rules that were not satisfied by the -// corresponding LockedProject in the Lock. -func (ls LockSatisfaction) UnmatchedOverrides() map[gps.ProjectRoot]ConstraintMismatch { - return ls.badovr -} - -// UnmatchedConstraints reports any normal, non-override constraint rules that -// were not satisfied by the corresponding LockedProject in the Lock. -func (ls LockSatisfaction) UnmatchedConstraints() map[gps.ProjectRoot]ConstraintMismatch { - return ls.badconstraint -} - func findEffectualConstraints(m gps.Manifest, imports map[string]bool) map[string]bool { eff := make(map[string]bool) xt := radix.New() diff --git a/gps/verify/locksat_test.go b/gps/verify/locksat_test.go new file mode 100644 index 0000000000..044b1f8b02 --- /dev/null +++ b/gps/verify/locksat_test.go @@ -0,0 +1,266 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package verify + +import ( + "strings" + "testing" + + "github.com/golang/dep/gps" + "github.com/golang/dep/gps/pkgtree" +) + +type lockUnsatisfactionDimension uint8 + +const ( + noLock lockUnsatisfactionDimension = 1 << iota + missingImports + excessImports + unmatchedOverrides + unmatchedConstraints +) + +func (lsd lockUnsatisfactionDimension) String() string { + var parts []string + for i := uint(0); i < 5; i++ { + if lsd&(1<