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

testing: de-globalize discard loggers #1165

Merged
merged 1 commit into from
Sep 18, 2017
Merged

testing: de-globalize discard loggers #1165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions context_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import (
"github.com/golang/dep/internal/test"
)

var (
discardLogger = log.New(ioutil.Discard, "", 0)
)
func discardLogger() *log.Logger {
Copy link
Collaborator

@carolynvs carolynvs Sep 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about moving this to internal/test and exporting? That way it doesn't need to be defined.

Copy link
Member

@sdboyer sdboyer Sep 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm kinda meh about that - tbh I was kinda meh about my original suggestion, but the idea of exporting it just makes it seem definitively unwise 😛

Copy link
Collaborator

@carolynvs carolynvs Sep 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't care much about if we do it or not but out of curiosity what's wrong/unwise with exporting test.DiscardLogger() in our internal test package?

Copy link
Member

@sdboyer sdboyer Sep 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a really subjective thing, which is why i sorta hesitate about it in the first place, but to my mind, exporting jumps the line between an innocuous helper and a recommended pattern.

return log.New(ioutil.Discard, "", 0)
}

func TestCtx_ProjectImport(t *testing.T) {
h := test.NewHelper(t)
Expand Down Expand Up @@ -127,8 +127,8 @@ func TestLoadProject(t *testing.T) {
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
ctx := &Ctx{
Out: discardLogger,
Err: discardLogger,
Out: discardLogger(),
Err: discardLogger(),
}

err := ctx.SetPaths(h.Path(tc.wd), h.Path("."))
Expand Down Expand Up @@ -200,8 +200,8 @@ func TestLoadProjectManifestParseError(t *testing.T) {
ctx := &Ctx{
GOPATH: tg.Path("."),
WorkingDir: wd,
Out: discardLogger,
Err: discardLogger,
Out: discardLogger(),
Err: discardLogger(),
}

_, err = ctx.LoadProject()
Expand Down Expand Up @@ -231,8 +231,8 @@ func TestLoadProjectLockParseError(t *testing.T) {
ctx := &Ctx{
GOPATH: tg.Path("."),
WorkingDir: wd,
Out: discardLogger,
Err: discardLogger,
Out: discardLogger(),
Err: discardLogger(),
}

_, err = ctx.LoadProject()
Expand Down
11 changes: 7 additions & 4 deletions internal/gps/result_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import (
"github.com/golang/dep/internal/test"
)

var discardLogger = log.New(ioutil.Discard, "", 0)
func discardLogger() *log.Logger {
return log.New(ioutil.Discard, "", 0)
}

var basicResult solution

Expand Down Expand Up @@ -95,12 +97,12 @@ func testWriteDepTree(t *testing.T) {
}

// nil lock/result should err immediately
err = WriteDepTree(tmp, nil, sm, true, discardLogger)
err = WriteDepTree(tmp, nil, sm, true, discardLogger())
if err == nil {
t.Errorf("Should error if nil lock is passed to WriteDepTree")
}

err = WriteDepTree(tmp, r, sm, true, discardLogger)
err = WriteDepTree(tmp, r, sm, true, discardLogger())
if err != nil {
t.Errorf("Unexpected error while creating vendor tree: %s", err)
}
Expand Down Expand Up @@ -143,6 +145,7 @@ func BenchmarkCreateVendorTree(b *testing.B) {
}

if clean {
logger := discardLogger()
b.ResetTimer()
b.StopTimer()
exp := path.Join(tmp, "export")
Expand All @@ -151,7 +154,7 @@ func BenchmarkCreateVendorTree(b *testing.B) {
// ease manual inspection
os.RemoveAll(exp)
b.StartTimer()
err = WriteDepTree(exp, r, sm, true, discardLogger)
err = WriteDepTree(exp, r, sm, true, logger)
b.StopTimer()
if err != nil {
b.Errorf("unexpected error after %v iterations: %s", i, err)
Expand Down
4 changes: 2 additions & 2 deletions test_project_context_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ func NewTestProjectContext(h *test.Helper, projectName string) *TestProjectConte
var err error
pc.Context = &Ctx{
GOPATH: pc.tempDir,
Out: discardLogger,
Err: discardLogger,
Out: discardLogger(),
Err: discardLogger(),
}
pc.SourceManager, err = pc.Context.SourceManager()
h.Must(errors.Wrap(err, "Unable to create a SourceManager"))
Expand Down
28 changes: 14 additions & 14 deletions txn_writer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestSafeWriter_BadInput_MissingRoot(t *testing.T) {
defer pc.Release()

sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged)
err := sw.Write("", pc.SourceManager, true, discardLogger)
err := sw.Write("", pc.SourceManager, true, discardLogger())

if err == nil {
t.Fatal("should have errored without a root path, but did not")
Expand All @@ -44,7 +44,7 @@ func TestSafeWriter_BadInput_MissingSourceManager(t *testing.T) {
pc.Load()

sw, _ := NewSafeWriter(nil, nil, pc.Project.Lock, VendorAlways)
err := sw.Write(pc.Project.AbsRoot, nil, true, discardLogger)
err := sw.Write(pc.Project.AbsRoot, nil, true, discardLogger())

if err == nil {
t.Fatal("should have errored without a source manager when forceVendor is true, but did not")
Expand Down Expand Up @@ -92,7 +92,7 @@ func TestSafeWriter_BadInput_NonexistentRoot(t *testing.T) {
sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged)

missingroot := filepath.Join(pc.Project.AbsRoot, "nonexistent")
err := sw.Write(missingroot, pc.SourceManager, true, discardLogger)
err := sw.Write(missingroot, pc.SourceManager, true, discardLogger())

if err == nil {
t.Fatal("should have errored with nonexistent dir for root path, but did not")
Expand All @@ -110,7 +110,7 @@ func TestSafeWriter_BadInput_RootIsFile(t *testing.T) {
sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged)

fileroot := pc.CopyFile("fileroot", "txn_writer/badinput_fileroot")
err := sw.Write(fileroot, pc.SourceManager, true, discardLogger)
err := sw.Write(fileroot, pc.SourceManager, true, discardLogger())

if err == nil {
t.Fatal("should have errored when root path is a file, but did not")
Expand Down Expand Up @@ -145,7 +145,7 @@ func TestSafeWriter_Manifest(t *testing.T) {
}

// Write changes
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger)
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger())
h.Must(errors.Wrap(err, "SafeWriter.Write failed"))

// Verify file system changes
Expand Down Expand Up @@ -190,7 +190,7 @@ func TestSafeWriter_ManifestAndUnmodifiedLock(t *testing.T) {
}

// Write changes
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger)
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger())
h.Must(errors.Wrap(err, "SafeWriter.Write failed"))

// Verify file system changes
Expand Down Expand Up @@ -235,7 +235,7 @@ func TestSafeWriter_ManifestAndUnmodifiedLockWithForceVendor(t *testing.T) {
}

// Write changes
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger)
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger())
h.Must(errors.Wrap(err, "SafeWriter.Write failed"))

// Verify file system changes
Expand Down Expand Up @@ -285,7 +285,7 @@ func TestSafeWriter_ModifiedLock(t *testing.T) {
}

// Write changes
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger)
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger())
h.Must(errors.Wrap(err, "SafeWriter.Write failed"))

// Verify file system changes
Expand Down Expand Up @@ -335,7 +335,7 @@ func TestSafeWriter_ModifiedLockSkipVendor(t *testing.T) {
}

// Write changes
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger)
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger())
h.Must(errors.Wrap(err, "SafeWriter.Write failed"))

// Verify file system changes
Expand Down Expand Up @@ -363,7 +363,7 @@ func TestSafeWriter_ForceVendorWhenVendorAlreadyExists(t *testing.T) {
pc.Load()

sw, _ := NewSafeWriter(nil, pc.Project.Lock, pc.Project.Lock, VendorAlways)
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger)
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger())
h.Must(errors.Wrap(err, "SafeWriter.Write failed"))

// Verify prepared actions
Expand All @@ -381,7 +381,7 @@ func TestSafeWriter_ForceVendorWhenVendorAlreadyExists(t *testing.T) {
t.Fatal("Expected the payload to contain the vendor directory ")
}

err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger)
err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger())
h.Must(errors.Wrap(err, "SafeWriter.Write failed"))

// Verify file system changes
Expand Down Expand Up @@ -431,7 +431,7 @@ func TestSafeWriter_NewLock(t *testing.T) {
}

// Write changes
err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger)
err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger())
h.Must(errors.Wrap(err, "SafeWriter.Write failed"))

// Verify file system changes
Expand Down Expand Up @@ -478,7 +478,7 @@ func TestSafeWriter_NewLockSkipVendor(t *testing.T) {
}

// Write changes
err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger)
err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger())
h.Must(errors.Wrap(err, "SafeWriter.Write failed"))

// Verify file system changes
Expand Down Expand Up @@ -571,7 +571,7 @@ func TestSafeWriter_VendorDotGitPreservedWithForceVendor(t *testing.T) {
t.Fatal("Expected the payload to contain the vendor directory")
}

err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger)
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger())
h.Must(errors.Wrap(err, "SafeWriter.Write failed"))

// Verify file system changes
Expand Down