From 996a18f2f53bc908b2aeab1323d8ce3112ebaa02 Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Fri, 11 Aug 2017 12:34:17 -0500 Subject: [PATCH] Apply common validations to all import conversion unit tests * Remove matchVersionPair flag and validate the cast anytime a * wantRevision was specified. * Always validate lock properties when a lock is generated. --- cmd/dep/glide_importer_test.go | 174 ++++++++++++--------------------- cmd/dep/godep_importer_test.go | 128 ++++++++---------------- cmd/dep/root_analyzer_test.go | 99 +++++++++++++++++++ 3 files changed, 202 insertions(+), 199 deletions(-) diff --git a/cmd/dep/glide_importer_test.go b/cmd/dep/glide_importer_test.go index ce624234bf..a16d044b32 100644 --- a/cmd/dep/glide_importer_test.go +++ b/cmd/dep/glide_importer_test.go @@ -36,18 +36,9 @@ func newTestContext(h *test.Helper) *dep.Ctx { func TestGlideConfig_Convert(t *testing.T) { testCases := map[string]struct { - yaml glideYaml - lock *glideLock - wantConvertErr bool - matchPairedVersion bool - projectRoot gps.ProjectRoot - wantSourceRepo string - wantConstraint string - wantRevision gps.Revision - wantVersion string - wantLockCount int - wantIgnoreCount int - wantIgnoredPackages []string + *convertTestCase + yaml glideYaml + lock *glideLock }{ "project": { yaml: glideYaml{ @@ -68,13 +59,14 @@ func TestGlideConfig_Convert(t *testing.T) { }, }, }, - projectRoot: "github.com/sdboyer/deptest", - wantSourceRepo: "https://github.com/sdboyer/deptest.git", - matchPairedVersion: true, - wantConstraint: "^1.0.0", - wantLockCount: 1, - wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), - wantVersion: "v1.0.0", + convertTestCase: &convertTestCase{ + projectRoot: "github.com/sdboyer/deptest", + wantSourceRepo: "https://github.com/sdboyer/deptest.git", + wantConstraint: "^1.0.0", + wantLockCount: 1, + wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), + wantVersion: "v1.0.0", + }, }, "test project": { yaml: glideYaml{ @@ -93,42 +85,74 @@ func TestGlideConfig_Convert(t *testing.T) { }, }, }, - projectRoot: "github.com/sdboyer/deptest", - wantLockCount: 1, - wantConstraint: "^1.0.0", - wantVersion: "v1.0.0", + convertTestCase: &convertTestCase{ + projectRoot: "github.com/sdboyer/deptest", + wantLockCount: 1, + wantConstraint: "^1.0.0", + wantVersion: "v1.0.0", + }, + }, + "revision only": { + yaml: glideYaml{ + Imports: []glidePackage{ + { + Name: "github.com/sdboyer/deptest", + }, + }, + }, + lock: &glideLock{ + Imports: []glideLockedPackage{ + { + Name: "github.com/sdboyer/deptest", + Reference: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + }, + }, + }, + convertTestCase: &convertTestCase{ + projectRoot: "github.com/sdboyer/deptest", + wantLockCount: 1, + wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), + }, }, "with ignored package": { yaml: glideYaml{ Ignores: []string{"github.com/sdboyer/deptest"}, }, - projectRoot: "github.com/sdboyer/deptest", - wantIgnoreCount: 1, - wantIgnoredPackages: []string{"github.com/sdboyer/deptest"}, + convertTestCase: &convertTestCase{ + projectRoot: "github.com/sdboyer/deptest", + wantIgnoreCount: 1, + wantIgnoredPackages: []string{"github.com/sdboyer/deptest"}, + }, }, "with exclude dir": { yaml: glideYaml{ ExcludeDirs: []string{"samples"}, }, - projectRoot: testGlideProjectRoot, - wantIgnoreCount: 1, - wantIgnoredPackages: []string{"github.com/golang/notexist/samples"}, + convertTestCase: &convertTestCase{ + projectRoot: testGlideProjectRoot, + wantIgnoreCount: 1, + wantIgnoredPackages: []string{"github.com/golang/notexist/samples"}, + }, }, "exclude dir ignores mismatched package name": { yaml: glideYaml{ Name: "github.com/golang/mismatched-package-name", ExcludeDirs: []string{"samples"}, }, - projectRoot: testGlideProjectRoot, - wantIgnoreCount: 1, - wantIgnoredPackages: []string{"github.com/golang/notexist/samples"}, + convertTestCase: &convertTestCase{ + projectRoot: testGlideProjectRoot, + wantIgnoreCount: 1, + wantIgnoredPackages: []string{"github.com/golang/notexist/samples"}, + }, }, "bad input, empty package name": { yaml: glideYaml{ Imports: []glidePackage{{Name: ""}}, }, - projectRoot: testGlideProjectRoot, - wantConvertErr: true, + convertTestCase: &convertTestCase{ + projectRoot: testGlideProjectRoot, + wantConvertErr: true, + }, }, } @@ -149,86 +173,10 @@ func TestGlideConfig_Convert(t *testing.T) { g.lock = testCase.lock } - manifest, lock, err := g.convert(testCase.projectRoot) + manifest, lock, convertErr := g.convert(testCase.projectRoot) + err := validateConvertTestCase(testCase.convertTestCase, manifest, lock, convertErr) if err != nil { - if testCase.wantConvertErr { - return - } - - t.Fatal(err) - } - - // Lock checks. - if lock != nil && len(lock.P) != testCase.wantLockCount { - t.Fatalf("Expected lock to have %d project(s), got %d", - testCase.wantLockCount, - len(lock.P)) - } - - // Ignored projects checks. - if len(manifest.Ignored) != testCase.wantIgnoreCount { - t.Fatalf("Expected manifest to have %d ignored project(s), got %d", - testCase.wantIgnoreCount, - len(manifest.Ignored)) - } - - if !equalSlice(manifest.Ignored, testCase.wantIgnoredPackages) { - t.Fatalf("Expected manifest to have ignore %s, got %s", - strings.Join(testCase.wantIgnoredPackages, ", "), - strings.Join(manifest.Ignored, ", ")) - } - - // Constraints checks below. Skip if there is no want constraint. - if testCase.wantConstraint == "" { - return - } - - d, ok := manifest.Constraints[testCase.projectRoot] - if !ok { - t.Fatalf("Expected the manifest to have a dependency for '%s' but got none", - testCase.projectRoot) - } - - v := d.Constraint.String() - if v != testCase.wantConstraint { - t.Fatalf("Expected manifest constraint to be %s, got %s", testCase.wantConstraint, v) - } - - p := lock.P[0] - - if p.Ident().ProjectRoot != testCase.projectRoot { - t.Fatalf("Expected the lock to have a project for '%s' but got '%s'", - testCase.projectRoot, - p.Ident().ProjectRoot) - } - - if p.Ident().Source != testCase.wantSourceRepo { - t.Fatalf("Expected locked source to be %s, got '%s'", testCase.wantSourceRepo, p.Ident().Source) - } - - lv := p.Version() - lpv, ok := lv.(gps.PairedVersion) - - if !ok { - if testCase.matchPairedVersion { - t.Fatalf("Expected locked version to be PairedVersion but got %T", lv) - } - - return - } - - ver := lpv.String() - if ver != testCase.wantVersion { - t.Fatalf("Expected locked version to be '%s', got %s", testCase.wantVersion, ver) - } - - if testCase.wantRevision != "" { - rev := lpv.Revision() - if rev != testCase.wantRevision { - t.Fatalf("Expected locked revision to be '%s', got %s", - testCase.wantRevision, - rev) - } + t.Fatalf("%+v", err) } }) } diff --git a/cmd/dep/godep_importer_test.go b/cmd/dep/godep_importer_test.go index 874455f22f..d242af573e 100644 --- a/cmd/dep/godep_importer_test.go +++ b/cmd/dep/godep_importer_test.go @@ -19,14 +19,8 @@ const testGodepProjectRoot = "github.com/golang/notexist" func TestGodepConfig_Convert(t *testing.T) { testCases := map[string]struct { - json godepJSON - wantConvertErr bool - matchPairedVersion bool - projectRoot gps.ProjectRoot - wantConstraint string - wantRevision gps.Revision - wantVersion string - wantLockCount int + *convertTestCase + json godepJSON }{ "convert project": { json: godepJSON{ @@ -39,12 +33,13 @@ func TestGodepConfig_Convert(t *testing.T) { }, }, }, - matchPairedVersion: true, - projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), - wantConstraint: "^0.8.0", - wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), - wantVersion: "v0.8.0", - wantLockCount: 1, + convertTestCase: &convertTestCase{ + projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), + wantConstraint: "^0.8.0", + wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), + wantVersion: "v0.8.0", + wantLockCount: 1, + }, }, "with semver suffix": { json: godepJSON{ @@ -56,11 +51,13 @@ func TestGodepConfig_Convert(t *testing.T) { }, }, }, - projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), - matchPairedVersion: false, - wantConstraint: "^1.12.0-12-g2fd980e", - wantLockCount: 1, - wantVersion: "v1.0.0", + convertTestCase: &convertTestCase{ + + projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), + wantConstraint: "^1.12.0-12-g2fd980e", + wantLockCount: 1, + wantVersion: "v1.0.0", + }, }, "empty comment": { json: godepJSON{ @@ -72,18 +69,23 @@ func TestGodepConfig_Convert(t *testing.T) { }, }, }, - projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), - matchPairedVersion: true, - wantConstraint: "^1.0.0", - wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), - wantVersion: "v1.0.0", - wantLockCount: 1, + convertTestCase: &convertTestCase{ + + projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), + wantConstraint: "^1.0.0", + wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), + wantVersion: "v1.0.0", + wantLockCount: 1, + }, }, "bad input - empty package name": { json: godepJSON{ Imports: []godepPackage{{ImportPath: ""}}, }, - wantConvertErr: true, + convertTestCase: &convertTestCase{ + + wantConvertErr: true, + }, }, "bad input - empty revision": { json: godepJSON{ @@ -93,7 +95,10 @@ func TestGodepConfig_Convert(t *testing.T) { }, }, }, - wantConvertErr: true, + convertTestCase: &convertTestCase{ + + wantConvertErr: true, + }, }, "sub-packages": { json: godepJSON{ @@ -110,10 +115,13 @@ func TestGodepConfig_Convert(t *testing.T) { }, }, }, - projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), - wantLockCount: 1, - wantConstraint: "^1.0.0", - wantVersion: "v1.0.0", + convertTestCase: &convertTestCase{ + + projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), + wantLockCount: 1, + wantConstraint: "^1.0.0", + wantVersion: "v1.0.0", + }, }, } @@ -130,62 +138,10 @@ func TestGodepConfig_Convert(t *testing.T) { g := newGodepImporter(discardLogger, true, sm) g.json = testCase.json - manifest, lock, err := g.convert(testCase.projectRoot) + manifest, lock, convertErr := g.convert(testCase.projectRoot) + err := validateConvertTestCase(testCase.convertTestCase, manifest, lock, convertErr) if err != nil { - if testCase.wantConvertErr { - return - } - - t.Fatal(err) - } - - if len(lock.P) != testCase.wantLockCount { - t.Fatalf("Expected lock to have %d project(s), got %d", - testCase.wantLockCount, - len(lock.P)) - } - - d, ok := manifest.Constraints[testCase.projectRoot] - if !ok { - t.Fatalf("Expected the manifest to have a dependency for '%s' but got none", - testCase.projectRoot) - } - - v := d.Constraint.String() - if v != testCase.wantConstraint { - t.Fatalf("Expected manifest constraint to be %s, got %s", testCase.wantConstraint, v) - } - - p := lock.P[0] - if p.Ident().ProjectRoot != testCase.projectRoot { - t.Fatalf("Expected the lock to have a project for '%s' but got '%s'", - testCase.projectRoot, - p.Ident().ProjectRoot) - } - - lv := p.Version() - lpv, ok := lv.(gps.PairedVersion) - - if !ok { - if testCase.matchPairedVersion { - t.Fatalf("Expected locked version to be PairedVersion but got %T", lv) - } - - return - } - - ver := lpv.String() - if ver != testCase.wantVersion { - t.Fatalf("Expected locked version to be '%s', got %s", testCase.wantVersion, ver) - } - - if testCase.wantRevision != "" { - rev := lpv.Revision() - if rev != testCase.wantRevision { - t.Fatalf("Expected locked revision to be '%s', got %s", - testCase.wantRevision, - rev) - } + t.Fatalf("%+v", err) } }) } diff --git a/cmd/dep/root_analyzer_test.go b/cmd/dep/root_analyzer_test.go index e06be921ca..f4003f016c 100644 --- a/cmd/dep/root_analyzer_test.go +++ b/cmd/dep/root_analyzer_test.go @@ -5,11 +5,13 @@ package main import ( + "strings" "testing" "github.com/golang/dep" "github.com/golang/dep/internal/gps" "github.com/golang/dep/internal/test" + "github.com/pkg/errors" ) func TestRootAnalyzer_Info(t *testing.T) { @@ -151,3 +153,100 @@ func TestProjectExistsInLock(t *testing.T) { }) } } + +// convertTestCase is a common set of validations applied to the result +// of an importer converting from an external config format to dep's. +type convertTestCase struct { + wantConvertErr bool + projectRoot gps.ProjectRoot + wantSourceRepo string + wantConstraint string + wantRevision gps.Revision + wantVersion string + wantLockCount int + wantIgnoreCount int + wantIgnoredPackages []string +} + +// validateConvertTestCase returns an error if any of the importer's +// conversion validations failed. +func validateConvertTestCase(testCase *convertTestCase, manifest *dep.Manifest, lock *dep.Lock, convertErr error) error { + if convertErr != nil { + if testCase.wantConvertErr { + return nil + } + + return convertErr + } + + // Ignored projects checks. + if len(manifest.Ignored) != testCase.wantIgnoreCount { + return errors.Errorf("Expected manifest to have %d ignored project(s), got %d", + testCase.wantIgnoreCount, + len(manifest.Ignored)) + } + + if !equalSlice(manifest.Ignored, testCase.wantIgnoredPackages) { + return errors.Errorf("Expected manifest to have ignore %s, got %s", + strings.Join(testCase.wantIgnoredPackages, ", "), + strings.Join(manifest.Ignored, ", ")) + } + + // Constraints checks below. + if testCase.wantConstraint != "" { + d, ok := manifest.Constraints[testCase.projectRoot] + if !ok { + return errors.Errorf("Expected the manifest to have a dependency for '%s' but got none", + testCase.projectRoot) + } + + v := d.Constraint.String() + if v != testCase.wantConstraint { + return errors.Errorf("Expected manifest constraint to be %s, got %s", testCase.wantConstraint, v) + } + } + + // Lock checks. + if lock != nil { + if len(lock.P) != testCase.wantLockCount { + return errors.Errorf("Expected lock to have %d project(s), got %d", + testCase.wantLockCount, + len(lock.P)) + } + + p := lock.P[0] + + if p.Ident().ProjectRoot != testCase.projectRoot { + return errors.Errorf("Expected the lock to have a project for '%s' but got '%s'", + testCase.projectRoot, + p.Ident().ProjectRoot) + } + + if p.Ident().Source != testCase.wantSourceRepo { + return errors.Errorf("Expected locked source to be %s, got '%s'", testCase.wantSourceRepo, p.Ident().Source) + } + + if testCase.wantVersion != "" { + ver := p.Version().String() + if ver != testCase.wantVersion { + return errors.Errorf("Expected locked version to be '%s', got %s", testCase.wantVersion, ver) + } + } + + if testCase.wantRevision != "" { + lv := p.Version() + lpv, ok := lv.(gps.PairedVersion) + if !ok { + return errors.Errorf("Expected locked version to be PairedVersion but got %T", lv) + } + + rev := lpv.Revision() + if rev != testCase.wantRevision { + return errors.Errorf("Expected locked revision to be '%s', got %s", + testCase.wantRevision, + rev) + } + } + } + return nil +}