Skip to content

Commit

Permalink
logparser: fix - consecutive changed imports are now coalesced into u…
Browse files Browse the repository at this point in the history 
…pdates.

Add more tests.
  • Loading branch information
Rob Figueiredo committed Sep 23, 2014
1 parent 2e44814 commit b45ba64
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 61 deletions.
122 changes: 93 additions & 29 deletions logparse_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package main
import (
"fmt"
"reflect"
"sort"
"strings"
"testing"
)

const TEMPLATE = `25b4da1 change lgo4go to bogus revision
const templateWithContext = `25b4da1 change lgo4go to bogus revision
diff --git a/REVISIONS b/REVISIONS
index 82ef4f5..efc84fa 100644
--- a/REVISIONS
Expand All @@ -21,84 +22,147 @@ index 82ef4f5..efc84fa 100644
github.com/codegangsta/martini/ 50f2d3f9d7eebef98b1a5dcf29dad72c66e918a7
`

const templateNoContext = `25b4da1 change lgo4go to bogus revision
diff --git a/REVISIONS b/REVISIONS
index 82ef4f5..efc84fa 100644
--- a/REVISIONS
+++ b/REVISIONS
@@ -1,6 +1,6 @@
%s
`

var tests = []struct {
name string
input string
diffs []string // one per "commit"
book playbook
}{
{"update existing", fmt.Sprintf(TEMPLATE, `
{"update existing", []string{`
-code.google.com/p/log4go c3294304d93f
+code.google.com/p/log4go 4794f7baff22
`), playbook{library: []libraryAction{
`}, playbook{library: []libraryAction{
{update, "code.google.com/p/log4go", "4794f7baff22"},
}}},

{"update existing 2", fmt.Sprintf(TEMPLATE, `
+code.google.com/p/log4go c3294304d93f
-code.google.com/p/log4go 4794f7baff22
`), playbook{library: []libraryAction{
{update, "code.google.com/p/log4go", "c3294304d93f"},
{"update existing (add first)", []string{`
+code.google.com/p/log4go 4794f7baff22
-code.google.com/p/log4go c3294304d93f
`}, playbook{library: []libraryAction{
{update, "code.google.com/p/log4go", "4794f7baff22"},
}}},

{"update existing block", []string{`
-code.google.com/p/log4go c3294304d93f
-code.google.com/p/log4go2 4794f7baff22
+code.google.com/p/log4go 4794f7baff22
+code.google.com/p/log4go2 c3294304d93f
`}, playbook{library: []libraryAction{
{update, "code.google.com/p/log4go", "4794f7baff22"},
{update, "code.google.com/p/log4go2", "c3294304d93f"},
}}},

{"add dep", fmt.Sprintf(TEMPLATE, `
{"update existing block (add first)", []string{`
+code.google.com/p/log4go 4794f7baff22
`), playbook{library: []libraryAction{
+code.google.com/p/log4go2 c3294304d93f
-code.google.com/p/log4go c3294304d93f
-code.google.com/p/log4go2 4794f7baff22
`}, playbook{library: []libraryAction{
{update, "code.google.com/p/log4go", "4794f7baff22"},
{update, "code.google.com/p/log4go2", "c3294304d93f"},
}}},

{"update existing block (intermixed)", []string{`
+code.google.com/p/log4go 4794f7baff22
-code.google.com/p/log4go c3294304d93f
+code.google.com/p/log4go2 c3294304d93f
-code.google.com/p/log4go2 4794f7baff22
`}, playbook{library: []libraryAction{
{update, "code.google.com/p/log4go", "4794f7baff22"},
{update, "code.google.com/p/log4go2", "c3294304d93f"},
}}},

{"update existing block (intermixed 2)", []string{`
+code.google.com/p/log4go 4794f7baff22
-code.google.com/p/log4go2 4794f7baff22
+code.google.com/p/log4go2 c3294304d93f
-code.google.com/p/log4go c3294304d93f
`}, playbook{library: []libraryAction{
{update, "code.google.com/p/log4go", "4794f7baff22"},
{update, "code.google.com/p/log4go2", "c3294304d93f"},
}}},

{"add dep", []string{`
+code.google.com/p/log4go 4794f7baff22
`}, playbook{library: []libraryAction{
{add, "code.google.com/p/log4go", "4794f7baff22"},
}}},

{"remove dep", fmt.Sprintf(TEMPLATE, `
{"remove dep", []string{`
-code.google.com/p/log4go 4794f7baff22
`), playbook{library: []libraryAction{
`}, playbook{library: []libraryAction{
{remove, "code.google.com/p/log4go", "4794f7baff22"},
}}},

{"no change", "", playbook{}},
{"no change", []string{""}, playbook{}},

{"multiple commits", fmt.Sprintf(TEMPLATE, `
{"multiple commits", []string{`
+code.google.com/p/log4go 4794f7baff22
`) + fmt.Sprintf(TEMPLATE, `
`, `
+launchpad.net/gocheck 45
`), playbook{library: []libraryAction{
`}, playbook{library: []libraryAction{
{add, "code.google.com/p/log4go", "4794f7baff22"},
{add, "launchpad.net/gocheck", "45"},
}}},

{"multiple commits on same import path", fmt.Sprintf(TEMPLATE, `
{"multiple commits on same import path", []string{`
-code.google.com/p/log4go 2
+code.google.com/p/log4go 3
`) + fmt.Sprintf(TEMPLATE, `
`, `
-code.google.com/p/log4go 1
+code.google.com/p/log4go 2
`), playbook{library: []libraryAction{
`}, playbook{library: []libraryAction{
{update, "code.google.com/p/log4go", "3"},
}}},

{"exotic repo names", fmt.Sprintf(TEMPLATE, `
{"exotic repo names", []string{`
+code.google.com/p/go.tools 1
+github.com/shurcooL/Go_Package-Store 1
`), playbook{library: []libraryAction{
`}, playbook{library: []libraryAction{
{add, "code.google.com/p/go.tools", "1"},
{add, "github.com/shurcooL/Go_Package-Store", "1"},
}}},

{"add cmd", fmt.Sprintf(TEMPLATE, `
{"add cmd", []string{`
+cmd code.google.com/p/go.tools/cmd/godoc
`), playbook{cmd: []cmdAction{
`}, playbook{cmd: []cmdAction{
{true, "code.google.com/p/go.tools/cmd/godoc"},
}}},

{"remove cmd", fmt.Sprintf(TEMPLATE, `
{"remove cmd", []string{`
-cmd code.google.com/p/go.tools/cmd/godoc
`), playbook{cmd: []cmdAction{
`}, playbook{cmd: []cmdAction{
{false, "code.google.com/p/go.tools/cmd/godoc"},
}}},
}

func TestLogParser(t *testing.T) {
for _, test := range tests {
actual := buildPlaybook(readDiffLines(strings.NewReader(test.input)))
if !reflect.DeepEqual(actual, test.book) {
t.Errorf("%v: expected %d, got %d", test.name, test.book, actual)
for _, tmpl := range []string{templateWithContext, templateNoContext} {
var input string
for _, diff := range test.diffs {
input += fmt.Sprintf(tmpl, strings.TrimSpace(diff))
}
actual := buildPlaybook(readDiffLines(strings.NewReader(input)))
// the library actions may be in any order, sort them.
sort.Sort(byLibImportPath(actual.library))
if !reflect.DeepEqual(actual, test.book) {
t.Errorf("%v: expected %v, got %v", test.name, test.book, actual)
}
}
}
}

type byLibImportPath []libraryAction

func (b byLibImportPath) Len() int { return len(b) }
func (b byLibImportPath) Swap(i, j int) { b[i], b[j] = b[j], b[i] }
func (b byLibImportPath) Less(i, j int) bool { return b[i].importPath < b[j].importPath }
83 changes: 51 additions & 32 deletions logparser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,54 +78,73 @@ func readDiffLines(reader io.Reader) []diff {
return diffs
}

func processDiffBlock(diffs []diff) []libraryAction {
// Assume:
// - An import path appears once or twice in a block. If twice, it was updated.
var importPathActions = make(map[string]libraryAction)
for _, this := range diffs {
// Calculate the new action for this import path.
// (Potentially this updates a previously recorded action from e.g. add to update)
if existing, ok := importPathActions[this.importPath]; ok {
if existing.action == update {
panic("unexpected: found import path " + this.importPath + " > 2 times in the diff")
}
importPathActions[this.importPath] = newUpdate(existing, this)
} else {
importPathActions[this.importPath] = newAddOrRemove(this)
}
}

// Build all the library actions
var result []libraryAction
for _, action := range importPathActions {
result = append(result, action)
}
return result
}

func buildPlaybook(diffs []diff) playbook {
// Convert diffs into actions. Since they may touch the same lines over
// multiple commits, keep track of import paths that we've added commands for,
// and only add the first.
var (
book playbook
block []diff
seenImportPaths = make(map[string]struct{})
)
for i := 0; i < len(diffs); i++ {
var this = diffs[i]
if this == emptyLine {
continue
}
if this.revision == "cmd" {
book.cmd = append(book.cmd, cmdAction{this.added, this.importPath})
continue
}
if _, ok := seenImportPaths[this.importPath]; ok {
continue
}
seenImportPaths[this.importPath] = struct{}{}

// Is this an updated line pair (which we treat as a unit)?
var next = emptyLine
if i < len(diffs)-1 {
next = diffs[i+1]
}
if next != emptyLine && next.importPath == this.importPath {
if this.added == next.added {
panic("most unexpected")
for _, this := range append(diffs, emptyLine) {
switch {
case this == emptyLine:
// Consume the past block of diffs. Blocks of diffs are separated by emptyLine.
// Assume:
// - Blocks are processed in reverse chronological order. If we've seen an
// import path in a previous block, ignore it entirely.
if block == nil {
continue
}
for _, libAction := range processDiffBlock(block) {
if _, ok := seenImportPaths[libAction.importPath]; ok {
continue
}
seenImportPaths[libAction.importPath] = struct{}{}
book.library = append(book.library, libAction)
}
book.library = append(book.library, newUpdate(this, next))
i++
continue
block = nil
case this.revision == "cmd":
book.cmd = append(book.cmd, cmdAction{this.added, this.importPath})
default:
block = append(block, this)
}

// If not, record this as an action.
book.library = append(book.library, newAddOrRemove(this))
}
return book
}

func newUpdate(a, b diff) libraryAction {
var added = a
func newUpdate(a libraryAction, b diff) libraryAction {
if b.added {
added = b
return newCommand(update, b)
}
return newCommand(update, added)
a.action = update
return a
}

func newAddOrRemove(d diff) libraryAction {
Expand Down

0 comments on commit b45ba64

Please sign in to comment.