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

Commit e8a2617

Browse files
darkowlzzdarkowlzz
committed
status: return slice of error from collectConstraints()
Errors from collectConstraints() should not result in failure of status. These errors are logged to stderr and the error count is added to `errCount` of `runStatusAll()`. This results in letting the user know about incomplete status result, keeping the status output clean. Running status in verbose mode would show all those errors.
1 parent 73ca5d9 commit e8a2617

File tree

2 files changed

+73
-55
lines changed

2 files changed

+73
-55
lines changed

‎cmd/dep/status.go‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,12 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana
417417
return false, 0, errors.Wrapf(err, "could not set up solver for input hashing")
418418
}
419419

420-
cm := collectConstraints(ctx, p, sm)
420+
// Errors while collecting constraints should not fail the whole status run.
421+
// It should count the error and tell the user about incomplete results.
422+
cm, ccerrs := collectConstraints(ctx, p, sm)
423+
if len(ccerrs) > 0 {
424+
errCount += len(ccerrs)
425+
}
421426

422427
// Get the project list and sort it so that the printed output users see is
423428
// deterministically ordered. (This may be superfluous if the lock is always
@@ -566,7 +571,7 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana
566571

567572
// Count ListVersions error because we get partial results when
568573
// this happens.
569-
errCount = len(errListVerCh)
574+
errCount += len(errListVerCh)
570575
if ctx.Verbose {
571576
for err := range errListVerCh {
572577
ctx.Err.Println(err.Error())
@@ -684,28 +689,31 @@ type projectConstraint struct {
684689
type constraintsCollection map[string][]projectConstraint
685690

686691
// collectConstraints collects constraints declared by all the dependencies.
687-
func collectConstraints(ctx *dep.Ctx, p *dep.Project, sm gps.SourceManager) constraintsCollection {
692+
// It returns constraintsCollection and a slice of errors encountered while
693+
// collecting the constraints, if any.
694+
func collectConstraints(ctx *dep.Ctx, p *dep.Project, sm gps.SourceManager) (constraintsCollection, []error) {
688695
logger := ctx.Err
689696
if !ctx.Verbose {
690697
logger = log.New(ioutil.Discard, "", 0)
691698
}
692699

693700
logger.Println("Collecting project constraints:")
694701

702+
var mutex sync.Mutex
703+
constraintCollection := make(constraintsCollection)
704+
695705
// Get direct deps of the root project.
696706
_, directDeps, err := getDirectDependencies(sm, p)
697707
if err != nil {
698-
logger.Println("Error getting direct deps:", err)
708+
// Return empty collection, not nil, if we fail here.
709+
return constraintCollection, []error{errors.Wrap(err, "failed to get direct dependencies")}
699710
}
700711

701712
// Create a root analyzer.
702713
rootAnalyzer := newRootAnalyzer(true, ctx, directDeps, sm)
703714

704715
lp := p.Lock.Projects()
705716

706-
var mutex sync.Mutex
707-
constraintCollection := make(constraintsCollection)
708-
709717
// Channel for receiving all the errors.
710718
errCh := make(chan error, len(lp))
711719

@@ -750,13 +758,15 @@ func collectConstraints(ctx *dep.Ctx, p *dep.Project, sm gps.SourceManager) cons
750758
wg.Wait()
751759
close(errCh)
752760

761+
var errs []error
753762
if len(errCh) > 0 {
754-
for err := range errCh {
755-
logger.Println(err.Error())
763+
for e := range errCh {
764+
errs = append(errs, e)
765+
logger.Println(e.Error())
756766
}
757767
}
758768

759-
return constraintCollection
769+
return constraintCollection, errs
760770
}
761771

762772
type byProject []projectConstraint

‎cmd/dep/status_test.go‎

Lines changed: 53 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"bytes"
99
"io/ioutil"
1010
"log"
11+
"path/filepath"
1112
"reflect"
1213
"testing"
1314
"text/tabwriter"
@@ -299,45 +300,42 @@ func TestCollectConstraints(t *testing.T) {
299300

300301
cases := []struct {
301302
name string
302-
project dep.Project
303+
lock dep.Lock
303304
wantConstraints constraintsCollection
305+
wantErr bool
304306
}{
305307
{
306308
name: "without any constraints",
307-
project: dep.Project{
308-
Lock: &dep.Lock{
309-
P: []gps.LockedProject{
310-
gps.NewLockedProject(
311-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")},
312-
gps.NewVersion("v1.0.0"),
313-
[]string{"."},
314-
),
315-
},
309+
lock: dep.Lock{
310+
P: []gps.LockedProject{
311+
gps.NewLockedProject(
312+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")},
313+
gps.NewVersion("v1.0.0"),
314+
[]string{"."},
315+
),
316316
},
317317
},
318318
wantConstraints: constraintsCollection{},
319319
},
320320
{
321321
name: "with multiple constraints",
322-
project: dep.Project{
323-
Lock: &dep.Lock{
324-
P: []gps.LockedProject{
325-
gps.NewLockedProject(
326-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")},
327-
gps.NewVersion("v1.0.0"),
328-
[]string{"."},
329-
),
330-
gps.NewLockedProject(
331-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-1")},
332-
gps.NewVersion("v0.1.0"),
333-
[]string{"."},
334-
),
335-
gps.NewLockedProject(
336-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-2")},
337-
gps.NewBranch("master").Pair(gps.Revision("824a8d56a4c6b2f4718824a98cd6d70d3dbd4c3e")),
338-
[]string{"."},
339-
),
340-
},
322+
lock: dep.Lock{
323+
P: []gps.LockedProject{
324+
gps.NewLockedProject(
325+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")},
326+
gps.NewVersion("v1.0.0"),
327+
[]string{"."},
328+
),
329+
gps.NewLockedProject(
330+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-1")},
331+
gps.NewVersion("v0.1.0"),
332+
[]string{"."},
333+
),
334+
gps.NewLockedProject(
335+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-2")},
336+
gps.NewBranch("master").Pair(gps.Revision("824a8d56a4c6b2f4718824a98cd6d70d3dbd4c3e")),
337+
[]string{"."},
338+
),
341339
},
342340
},
343341
wantConstraints: constraintsCollection{
@@ -355,27 +353,26 @@ func TestCollectConstraints(t *testing.T) {
355353
},
356354
{
357355
name: "skip projects with invalid versions",
358-
project: dep.Project{
359-
Lock: &dep.Lock{
360-
P: []gps.LockedProject{
361-
gps.NewLockedProject(
362-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-1")},
363-
gps.NewVersion("v0.1.0"),
364-
[]string{"."},
365-
),
366-
gps.NewLockedProject(
367-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-2")},
368-
gps.NewVersion("v1.0.0"),
369-
[]string{"."},
370-
),
371-
},
356+
lock: dep.Lock{
357+
P: []gps.LockedProject{
358+
gps.NewLockedProject(
359+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-1")},
360+
gps.NewVersion("v0.1.0"),
361+
[]string{"."},
362+
),
363+
gps.NewLockedProject(
364+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-2")},
365+
gps.NewVersion("v1.0.0"),
366+
[]string{"."},
367+
),
372368
},
373369
},
374370
wantConstraints: constraintsCollection{
375371
"github.com/sdboyer/deptest": []projectConstraint{
376372
{"github.com/darkowlzz/deptest-project-1", ver1},
377373
},
378374
},
375+
wantErr: true,
379376
},
380377
}
381378

@@ -396,12 +393,23 @@ func TestCollectConstraints(t *testing.T) {
396393
h.Must(err)
397394
defer sm.Release()
398395

396+
// Create new project and set root. Setting root is required for PackageList
397+
// to run properly.
398+
p := new(dep.Project)
399+
p.SetRoot(filepath.Join(pwd, "src"))
400+
399401
for _, c := range cases {
400402
t.Run(c.name, func(t *testing.T) {
401-
gotConstraints := collectConstraints(ctx, &c.project, sm)
403+
p.Lock = &c.lock
404+
gotConstraints, err := collectConstraints(ctx, p, sm)
405+
if len(err) > 0 && !c.wantErr {
406+
t.Fatalf("unexpected errors while collecting constraints: %v", err)
407+
} else if len(err) == 0 && c.wantErr {
408+
t.Fatalf("expected errors while collecting constraints, but got none")
409+
}
402410

403411
if !reflect.DeepEqual(gotConstraints, c.wantConstraints) {
404-
t.Fatalf("Unexpected collected constraints: \n\t(GOT): %v\n\t(WNT): %v", gotConstraints, c.wantConstraints)
412+
t.Fatalf("unexpected collected constraints: \n\t(GOT): %v\n\t(WNT): %v", gotConstraints, c.wantConstraints)
405413
}
406414
})
407415
}

0 commit comments

Comments
 (0)