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

[WIP] VerifyDepTree and DigestFromDirectory use godirwalk #1084

Closed
wants to merge 30 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
502d835
digest uses godirwalk
karrick Aug 21, 2017
f149f35
more simplified node type detection
karrick Aug 29, 2017
6dd14f9
digestNoAction constant name describes purpose of values
karrick Aug 29, 2017
157c30f
updated godirwalk
karrick Aug 30, 2017
ab1cacf
updated godirwalk
karrick Aug 30, 2017
4109fb9
update godirwalk to cleaner version v1.2.4
karrick Aug 31, 2017
be2c05c
eliminated unused variables
karrick Aug 31, 2017
ac6f562
vendor'd package in accordance with guidance from travis ci
karrick Aug 31, 2017
e058b2f
Don't assume every git repository has a HEAD
BartSchuurmans Aug 23, 2017
8a131bd
Add test for gitSource.listVersions() without HEAD
BartSchuurmans Sep 3, 2017
caca12a
cmd/dep: Standardize import logic for importers
carolynvs Aug 29, 2017
41d353e
cmd/dep: Support new importer rules
carolynvs Aug 31, 2017
d61e44e
cmd/dep: Switch to special importer testdata set
carolynvs Aug 31, 2017
fa8e165
cmd/dep: Move import test execution into testcase
carolynvs Sep 1, 2017
acd473f
cmd/dep: Validate warnings in import testcases
carolynvs Sep 1, 2017
f0e0e3a
cmd/dep: Document every baseImporter function
carolynvs Sep 5, 2017
7d5628c
cmd/dep: Simplify pinned constraint check
carolynvs Sep 5, 2017
7e85ef6
cmd/dep: fix parallel tests
carolynvs Sep 6, 2017
49a2eb4
cmd/dep: verify empty locks are skipped
carolynvs Sep 6, 2017
0321346
gps: source cache: fix flaky bolt test clock by forcing future timestamp
jmank88 Sep 6, 2017
94dec0f
Fix naming convention for test
BartSchuurmans Sep 7, 2017
59d1d75
Nicer error reporting in test
BartSchuurmans Sep 7, 2017
5332f3a
Don't rely on local git config in test
BartSchuurmans Sep 7, 2017
f380ff7
Fix test when git is not set up
BartSchuurmans Sep 8, 2017
5c9e128
Fix file:// URI on Windows
BartSchuurmans Sep 8, 2017
3d84357
gps: fix unwrapVcsErr to handle nil causes
jmank88 Sep 9, 2017
4626497
gps: add unwrapVcsErr nil cause test
jmank88 Sep 9, 2017
5e17c34
cmd/dep: fix setting importer tests as parallel
carolynvs Sep 9, 2017
a1f23c2
cmd/dep: disable running importer tests in parallel
carolynvs Sep 9, 2017
b40a93c
Clarify when to run ensure if vendor is not committed
carolynvs Aug 28, 2017
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
8 changes: 7 additions & 1 deletion Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,7 @@
[[constraint]]
name = "github.com/boltdb/bolt"
version = "1.0.0"

[[constraint]]
name = "github.com/karrick/godirwalk"
version = "1.2.7"
296 changes: 93 additions & 203 deletions internal/gps/pkgtree/digest.go

Large diffs are not rendered by default.

128 changes: 29 additions & 99 deletions internal/gps/pkgtree/digest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,96 +6,15 @@ package pkgtree

import (
"bytes"
"io"
"os"
"path/filepath"
"testing"
)

// crossBuffer is a test io.Reader that emits a few canned responses.
type crossBuffer struct {
readCount int
iterations []string
}

func (cb *crossBuffer) Read(buf []byte) (int, error) {
if cb.readCount == len(cb.iterations) {
return 0, io.EOF
}
cb.readCount++
return copy(buf, cb.iterations[cb.readCount-1]), nil
}

func streamThruLineEndingReader(t *testing.T, iterations []string) []byte {
dst := new(bytes.Buffer)
n, err := io.Copy(dst, newLineEndingReader(&crossBuffer{iterations: iterations}))
if got, want := err, error(nil); got != want {
t.Errorf("(GOT): %v; (WNT): %v", got, want)
}
if got, want := n, int64(dst.Len()); got != want {
t.Errorf("(GOT): %v; (WNT): %v", got, want)
}
return dst.Bytes()
}

func TestLineEndingReader(t *testing.T) {
testCases := []struct {
input []string
output string
}{
{[]string{"\r"}, "\r"},
{[]string{"\r\n"}, "\n"},
{[]string{"now is the time\r\n"}, "now is the time\n"},
{[]string{"now is the time\r\n(trailing data)"}, "now is the time\n(trailing data)"},
{[]string{"now is the time\n"}, "now is the time\n"},
{[]string{"now is the time\r"}, "now is the time\r"}, // trailing CR ought to convey
{[]string{"\rnow is the time"}, "\rnow is the time"}, // CR not followed by LF ought to convey
{[]string{"\rnow is the time\r"}, "\rnow is the time\r"}, // CR not followed by LF ought to convey

// no line splits
{[]string{"first", "second", "third"}, "firstsecondthird"},

// 1->2 and 2->3 both break across a CRLF
{[]string{"first\r", "\nsecond\r", "\nthird"}, "first\nsecond\nthird"},

// 1->2 breaks across CRLF and 2->3 does not
{[]string{"first\r", "\nsecond", "third"}, "first\nsecondthird"},

// 1->2 breaks across CRLF and 2 ends in CR but 3 does not begin LF
{[]string{"first\r", "\nsecond\r", "third"}, "first\nsecond\rthird"},

// 1 ends in CR but 2 does not begin LF, and 2->3 breaks across CRLF
{[]string{"first\r", "second\r", "\nthird"}, "first\rsecond\nthird"},

// 1 ends in CR but 2 does not begin LF, and 2->3 does not break across CRLF
{[]string{"first\r", "second\r", "\nthird"}, "first\rsecond\nthird"},

// 1->2 and 2->3 both break across a CRLF, but 3->4 does not
{[]string{"first\r", "\nsecond\r", "\nthird\r", "fourth"}, "first\nsecond\nthird\rfourth"},
{[]string{"first\r", "\nsecond\r", "\nthird\n", "fourth"}, "first\nsecond\nthird\nfourth"},

{[]string{"this is the result\r\nfrom the first read\r", "\nthis is the result\r\nfrom the second read\r"},
"this is the result\nfrom the first read\nthis is the result\nfrom the second read\r"},
{[]string{"now is the time\r\nfor all good engineers\r\nto improve their test coverage!\r\n"},
"now is the time\nfor all good engineers\nto improve their test coverage!\n"},
{[]string{"now is the time\r\nfor all good engineers\r", "\nto improve their test coverage!\r\n"},
"now is the time\nfor all good engineers\nto improve their test coverage!\n"},
}

for _, testCase := range testCases {
got := streamThruLineEndingReader(t, testCase.input)
if want := []byte(testCase.output); !bytes.Equal(got, want) {
t.Errorf("Input: %#v; (GOT): %#q; (WNT): %#q", testCase.input, got, want)
}
}
}

////////////////////////////////////////

func getTestdataVerifyRoot(t *testing.T) string {
func getTestdataVerifyRoot(tb testing.TB) string {
cwd, err := os.Getwd()
if err != nil {
t.Fatal(err)
tb.Fatal(err)
}
return filepath.Join(filepath.Dir(cwd), "testdata_digest")
}
Expand Down Expand Up @@ -139,6 +58,32 @@ func TestDigestFromDirectory(t *testing.T) {
})
}

func BenchmarkDigestFromDirectory(b *testing.B) {
prefix := getTestdataVerifyRoot(b)
b.ResetTimer()

for i := 0; i < b.N; i++ {
_, err := DigestFromDirectory(prefix)
if err != nil {
b.Fatal(err)
}
}
}

const flameIterations = 100000
Copy link
Member

Choose a reason for hiding this comment

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

could you explain the benefit of having a fixed number of iterations, rather than relying (as BenchmarkDigestFromDirectory() does) on the testing system to either a) empirically determine an appropriate threshold and/or b) allow the user to control it via CLI args?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When comparing flame graphs, I want to compare how many cycles were spent in the corresponding functions for the same input. Those numbers are skewed if we run version A 500 times, and version B only 100 times. Therefore, forcing the iteration count to a particular number ensures an apples to apples comparison.

Copy link
Member

Choose a reason for hiding this comment

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

gotcha, wfm 👍


func BenchmarkFlameDigestFromDirectory(b *testing.B) {
prefix := getTestdataVerifyRoot(b)
b.ResetTimer()

for i := 0; i < flameIterations; i++ {
_, err := DigestFromDirectory(prefix)
if err != nil {
b.Fatal(err)
}
}
}

func TestVerifyDepTree(t *testing.T) {
vendorRoot := getTestdataVerifyRoot(t)

Expand Down Expand Up @@ -193,23 +138,8 @@ func TestVerifyDepTree(t *testing.T) {
}
}

func BenchmarkDigestFromDirectory(b *testing.B) {
b.Skip("Eliding benchmark of user's Go source directory")

prefix := filepath.Join(os.Getenv("GOPATH"), "src")

for i := 0; i < b.N; i++ {
_, err := DigestFromDirectory(prefix)
if err != nil {
b.Fatal(err)
}
}
}

func BenchmarkVerifyDepTree(b *testing.B) {
b.Skip("Eliding benchmark of user's Go source directory")

prefix := filepath.Join(os.Getenv("GOPATH"), "src")
prefix := getTestdataVerifyRoot(b)

for i := 0; i < b.N; i++ {
_, err := VerifyDepTree(prefix, nil)
Expand Down
139 changes: 0 additions & 139 deletions internal/gps/pkgtree/dirwalk.go

This file was deleted.

Loading