Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Commit

Permalink
cmd/dep: Validate warnings in import testcases
Browse files Browse the repository at this point in the history
  • Loading branch information
carolynvs committed Sep 5, 2017
1 parent 2d44886 commit 1407196
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 51 deletions.
6 changes: 2 additions & 4 deletions cmd/dep/base_importer.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,6 @@ func (i *baseImporter) importPackages(packages []importedPackage, defaultConstra
pc.Constraint, err = i.sm.InferConstraint(prj.ConstraintHint, pc.Ident)
if err != nil {
pc.Constraint = gps.Any()
err = errors.Wrapf(err, "Unable to interpret constraint '%s' for package %s. Using the 'any' constraint instead.")
i.logger.Println(err)
}

var version gps.Version
Expand Down Expand Up @@ -224,7 +222,7 @@ func (i *baseImporter) importPackages(packages []importedPackage, defaultConstra
// Ignore pinned constraints
if i.isConstraintPinned(pc.Constraint) {
if i.verbose {
i.logger.Printf("Ignoring pinned constraint %v for %v.\n", pc.Constraint, pc.Ident)
i.logger.Printf(" Ignoring pinned constraint %v for %v.\n", pc.Constraint, pc.Ident)
}
pc.Constraint = gps.Any()
}
Expand All @@ -233,7 +231,7 @@ func (i *baseImporter) importPackages(packages []importedPackage, defaultConstra
// solve doesn't later change the revision to satisfy the constraint.
if !i.testConstraint(pc.Constraint, version) {
if i.verbose {
i.logger.Printf("Ignoring constraint %v for %v because it would invalidate the locked version %v.\n", pc.Constraint, pc.Ident, version)
i.logger.Printf(" Ignoring constraint %v for %v because it would invalidate the locked version %v.\n", pc.Constraint, pc.Ident, version)
}
pc.Constraint = gps.Any()
}
Expand Down
16 changes: 12 additions & 4 deletions cmd/dep/base_importer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"bytes"
"log"
"sort"
"strings"
"testing"

"github.com/golang/dep"
Expand Down Expand Up @@ -397,6 +398,7 @@ type convertTestCase struct {
wantRevision gps.Revision
wantVersion string
wantIgnored []string
wantWarning string
}

func (tc convertTestCase) Exec(t *testing.T, convert func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock, error)) error {
Expand All @@ -409,15 +411,15 @@ func (tc convertTestCase) Exec(t *testing.T, convert func(logger *log.Logger, sm
defer sm.Release()

// Capture stderr so we can verify warnings
verboseOutput := &bytes.Buffer{}
ctx.Err = log.New(verboseOutput, "", 0)
output := &bytes.Buffer{}
ctx.Err = log.New(output, "", 0)

manifest, lock, convertErr := convert(ctx.Err, sm)
return tc.validate(manifest, lock, convertErr)
return tc.validate(manifest, lock, convertErr, output)
}

// validate returns an error if any of the testcase validations failed.
func (tc convertTestCase) validate(manifest *dep.Manifest, lock *dep.Lock, convertErr error) error {
func (tc convertTestCase) validate(manifest *dep.Manifest, lock *dep.Lock, convertErr error, output *bytes.Buffer) error {
if tc.wantConvertErr {
if convertErr == nil {
return errors.New("Expected the conversion to fail, but it did not return an error")
Expand Down Expand Up @@ -512,6 +514,12 @@ func (tc convertTestCase) validate(manifest *dep.Manifest, lock *dep.Lock, conve
}
}

if tc.wantWarning != "" {
if !strings.Contains(output.String(), tc.wantWarning) {
return errors.Errorf("Expected the output to include the warning '%s'", tc.wantWarning)
}
}

return nil
}

Expand Down
71 changes: 28 additions & 43 deletions cmd/dep/glide_importer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"io/ioutil"
"log"
"path/filepath"
"strings"
"testing"

"github.com/golang/dep"
Expand Down Expand Up @@ -143,6 +142,34 @@ func TestGlideConfig_Convert(t *testing.T) {
wantConvertErr: true,
},
},
"warn unused os field": {
glideYaml{
Imports: []glidePackage{
{
Name: importerTestProject,
OS: "windows",
},
}},
glideLock{},
convertTestCase{
wantConstraint: "*",
wantWarning: "specified an os",
},
},
"warn unused arch field": {
glideYaml{
Imports: []glidePackage{
{
Name: importerTestProject,
Arch: "i686",
},
}},
glideLock{},
convertTestCase{
wantConstraint: "*",
wantWarning: "specified an arch",
},
},
}

for name, testCase := range testCases {
Expand Down Expand Up @@ -209,45 +236,3 @@ func TestGlideConfig_Import(t *testing.T) {
}
}
}

func TestGlideConfig_Convert_WarnsForUnusedFields(t *testing.T) {
testCases := map[string]glidePackage{
"specified an os": {OS: "windows"},
"specified an arch": {Arch: "i686"},
}

for wantWarning, pkg := range testCases {
t.Run(wantWarning, func(t *testing.T) {
h := test.NewHelper(t)
defer h.Cleanup()
h.Parallel()

pkg.Name = "github.com/sdboyer/deptest"
pkg.Reference = "v1.0.0"

ctx := newTestContext(h)
sm, err := ctx.SourceManager()
h.Must(err)
defer sm.Release()

// Capture stderr so we can verify warnings
verboseOutput := &bytes.Buffer{}
ctx.Err = log.New(verboseOutput, "", 0)

g := newGlideImporter(ctx.Err, true, sm)
g.glideConfig = glideYaml{
Imports: []glidePackage{pkg},
}

_, _, err = g.convert(testProjectRoot)
if err != nil {
t.Fatal(err)
}

warnings := verboseOutput.String()
if !strings.Contains(warnings, wantWarning) {
t.Errorf("Expected the output to include the warning '%s'", wantWarning)
}
})
}
}

0 comments on commit 1407196

Please sign in to comment.