From c4e318c85a0ffef8d6bb3a4525adba6dd6cddb4d Mon Sep 17 00:00:00 2001 From: Scott Nichols Date: Fri, 11 Mar 2022 14:43:45 -0800 Subject: [PATCH] better findby, split on comma, add find tracker Signed-off-by: Scott Nichols --- pkg/commands/diff.go | 12 +++ pkg/diff/diff.go | 168 +++++++++++++++++++------------ pkg/diff/diff_test.go | 14 +++ pkg/diff/testdata/sample7_a.yaml | 3 + pkg/diff/testdata/sample7_b.yaml | 33 ++++++ pkg/diff/testdata/sample8_a.yaml | 7 ++ pkg/diff/testdata/sample8_b.yaml | 65 ++++++++++++ 7 files changed, 239 insertions(+), 63 deletions(-) create mode 100644 pkg/diff/testdata/sample7_a.yaml create mode 100644 pkg/diff/testdata/sample7_b.yaml create mode 100644 pkg/diff/testdata/sample8_a.yaml create mode 100644 pkg/diff/testdata/sample8_b.yaml diff --git a/pkg/commands/diff.go b/pkg/commands/diff.go index 51e65f7..0ce41c1 100644 --- a/pkg/commands/diff.go +++ b/pkg/commands/diff.go @@ -7,6 +7,7 @@ package commands import ( "os" + "strings" "github.com/spf13/cobra" @@ -24,6 +25,17 @@ func addDiff(topLevel *cobra.Command) { cloudevents diff ./want.yaml ./got.yaml `, Args: cobra.ExactArgs(2), + PreRun: func(cmd *cobra.Command, args []string) { + var findBy []string + for _, fb := range do.FindBy { + if strings.Contains(fb, ",") { + findBy = append(findBy, strings.Split(fb, ",")...) + } else { + findBy = append(findBy, fb) + } + } + do.FindBy = findBy + }, Run: func(cmd *cobra.Command, args []string) { r := diff.Diff{ Out: cmd.OutOrStdout(), diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 5e6cd69..a2f85e5 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -8,13 +8,14 @@ package diff import ( "errors" "fmt" + cmpdiff "github.com/kylelemons/godebug/diff" "io" + "math" "strings" "github.com/cloudevents/conformance/pkg/event" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - cmpdiff "github.com/kylelemons/godebug/diff" ) // Diff compares two CloudEvents yaml files (or directories) for differences, @@ -30,9 +31,28 @@ type Diff struct { FileB string } -func (i *Diff) find(like event.Event, all []event.Event) (string, *event.Event) { +var ignoreOpts = cmpopts.IgnoreFields(event.Event{}, "Mode", "TransportExtensions") + +func newTracker(findBy []string, all []event.Event) *tracker { + return &tracker{ + findBy: findBy, + all: all, + used: make(map[int]bool, len(all)), + } +} + +type tracker struct { + findBy []string + all []event.Event + used map[int]bool +} + +type trackerFn func(string, event.Event) + +func (t *tracker) findBest(like event.Event, fn trackerFn) (string, error) { + // Make the label. label := "" - for _, b := range i.FindBy { + for _, b := range t.findBy { if len(label) == 0 { label = fmt.Sprintf("%s[%s]", b, like.Get(b)) } else { @@ -40,30 +60,49 @@ func (i *Diff) find(like event.Event, all []event.Event) (string, *event.Event) } } - // Look for the matching ID, Source and Subject. - for _, a := range all { + best := -1 + bestDiff := math.MaxInt + + // Look for like. + for i, b := range t.all { + if t.used[i] { + continue + } + + // Test for a match. match := true - for _, key := range i.FindBy { - if a.Get(key) != like.Get(key) { + for _, key := range t.findBy { + if like.Get(key) != b.Get(key) { match = false break } } - if match { - return label, &a + if !match { + continue + } + // Simple compute the "delta" between line and b, just the length of the diff. + diff := len(cmp.Diff(like, b, ignoreOpts)) + if diff < bestDiff { + best = i + bestDiff = diff } } - return label, nil + if best >= 0 { + fn(label, t.all[best]) + t.used[best] = true + return label, nil + } + return label, io.EOF } func (i *Diff) Do() error { - ignoreOpts := cmpopts.IgnoreFields(event.Event{}, "Mode", "TransportExtensions") - + // Read and parse FileA eventsA, err := event.FromYaml(i.FileA, true) if err != nil { return err } + // Read and parse FileB eventsB, err := event.FromYaml(i.FileB, true) if err != nil { return err @@ -71,68 +110,71 @@ func (i *Diff) Do() error { var diffs []string - for _, a := range eventsA { - label, b := i.find(a, eventsB) - - if b == nil { - _, _ = fmt.Fprintf(i.Out, "missing: %s\n", label) - diffs = append(diffs, fmt.Sprintf("missing: %s\n", label)) - continue - } + // Tracker helps manage selecting events based on findBy and the current event to compare. + t := newTracker(i.FindBy, eventsB) - if diff := cmp.Diff(a, b, ignoreOpts); diff != "" { - // Clear out Mode and Transport Extensions. - a.Mode = "" - b.Mode = "" - a.TransportExtensions = nil - b.TransportExtensions = nil + for _, a := range eventsA { - ab, err := event.ToYaml(a) - if err != nil { - return err - } - bb, err := event.ToYaml(*b) - if err != nil { - return err - } + if label, err := t.findBest(a, func(label string, b event.Event) { + if diff := cmp.Diff(a, b, ignoreOpts); diff != "" { + // Clear out Mode and Transport Extensions. + a.Mode = "" + b.Mode = "" + a.TransportExtensions = nil + b.TransportExtensions = nil + + ab, err := event.ToYaml(a) + if err != nil { + return + } + bb, err := event.ToYaml(b) + if err != nil { + return + } - ignore := true - sb := &strings.Builder{} - if len(diffs) >= 1 { - sb.WriteString("---\n") - } - sb.WriteString(fmt.Sprintf("%s diffs (-a, +b):\n", label)) - - chunks := cmpdiff.DiffChunks(strings.Split(string(ab), "\n"), strings.Split(string(bb), "\n")) - changed := map[string]bool{} - for _, c := range chunks { - if len(c.Added) != 0 { - for _, a := range c.Added { - if _, found := changed[strings.Split(a, ":")[0]]; found || !i.IgnoreAdditions { - ignore = false - sb.WriteString(fmt.Sprintf("+ %s\n", a)) + ignore := true + sb := &strings.Builder{} + if len(diffs) >= 1 { + sb.WriteString("---\n") + } + sb.WriteString(fmt.Sprintf("%s diffs (-a, +b):\n", label)) + + chunks := cmpdiff.DiffChunks(strings.Split(string(ab), "\n"), strings.Split(string(bb), "\n")) + changed := map[string]bool{} + for _, c := range chunks { + if len(c.Added) != 0 { + for _, a := range c.Added { + if _, found := changed[strings.Split(a, ":")[0]]; found || !i.IgnoreAdditions { + ignore = false + sb.WriteString(fmt.Sprintf("+ %s\n", a)) + } } } - } - if len(c.Deleted) != 0 { - ignore = false - for _, d := range c.Deleted { - sb.WriteString(fmt.Sprintf("- %s\n", d)) - changed[strings.Split(d, ":")[0]] = true + if len(c.Deleted) != 0 { + ignore = false + for _, d := range c.Deleted { + sb.WriteString(fmt.Sprintf("- %s\n", d)) + changed[strings.Split(d, ":")[0]] = true + } } } - } - if i.FullDiff { - _, _ = fmt.Fprintf(i.Out, "%s\n", cmpdiff.Diff(string(ab), string(bb))) - } else if !ignore { - _, _ = fmt.Fprint(i.Out, sb.String()) - if len(diffs) > 0 { - diffs = append(diffs, "---") + if i.FullDiff { + _, _ = fmt.Fprintf(i.Out, "%s\n", cmpdiff.Diff(string(ab), string(bb))) + } else if !ignore { + _, _ = fmt.Fprint(i.Out, sb.String()) + if len(diffs) > 0 { + diffs = append(diffs, "---") + } + diffs = append(diffs, cmpdiff.Diff(string(ab), string(bb))) } - diffs = append(diffs, cmpdiff.Diff(string(ab), string(bb))) } + }); err != nil { + _, _ = fmt.Fprintf(i.Out, "missing: %s\n", label) + diffs = append(diffs, fmt.Sprintf("missing: %s\n", label)) + continue } + } if len(diffs) > 0 { diff --git a/pkg/diff/diff_test.go b/pkg/diff/diff_test.go index 4aa8c46..a69daf3 100644 --- a/pkg/diff/diff_test.go +++ b/pkg/diff/diff_test.go @@ -64,6 +64,20 @@ type[com.example.someevent.c] diffs (-a, +b): - source: /mycontext/subcontext + source: /mycontext/subcontext2 `, + }, { + name: "sample7: two similar events, match second", + a: "./testdata/sample7_a.yaml", + b: "./testdata/sample7_b.yaml", + findBy: []string{"type", "subject"}, + ignoreAdditions: true, + wantErr: false, + }, { + name: "sample7: two similar events, two matches out of order, search just by type", + a: "./testdata/sample7_a.yaml", + b: "./testdata/sample7_b.yaml", + findBy: []string{"type"}, + ignoreAdditions: true, + wantErr: false, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/diff/testdata/sample7_a.yaml b/pkg/diff/testdata/sample7_a.yaml new file mode 100644 index 0000000..a24b5a3 --- /dev/null +++ b/pkg/diff/testdata/sample7_a.yaml @@ -0,0 +1,3 @@ +ContextAttributes: + type: com.example.someevent + subject: second-subject diff --git a/pkg/diff/testdata/sample7_b.yaml b/pkg/diff/testdata/sample7_b.yaml new file mode 100644 index 0000000..fdccb6d --- /dev/null +++ b/pkg/diff/testdata/sample7_b.yaml @@ -0,0 +1,33 @@ +Mode: structured +ContextAttributes: + specversion: 1.0 + type: com.example.someevent + time: 2018-04-05T03:56:14Z + id: 4321-4321-4321-a + source: /mycontext/subcontext + subject: first-subject + Extensions: + comexampleextension1 : "value" + comexampleextension2 : | + {"othervalue": 5} +TransportExtensions: + user-agent: "foo" +Data: | + {"world":"hello"} +--- +Mode: structured +ContextAttributes: + specversion: 1.0 + type: com.example.someevent + time: 2018-04-05T03:56:24Z + id: 4321-4321-4321-b + source: /mycontext/subcontext + subject: second-subject + Extensions: + comexampleextension1 : "value" + comexampleextension2 : | + {"othervalue": 5} +TransportExtensions: + user-agent: "foo" +Data: | + {"world":"hello"} diff --git a/pkg/diff/testdata/sample8_a.yaml b/pkg/diff/testdata/sample8_a.yaml new file mode 100644 index 0000000..90715bc --- /dev/null +++ b/pkg/diff/testdata/sample8_a.yaml @@ -0,0 +1,7 @@ +ContextAttributes: + type: com.example.someevent + subject: second-subject +--- +ContextAttributes: + type: com.example.someevent + subject: first-subject diff --git a/pkg/diff/testdata/sample8_b.yaml b/pkg/diff/testdata/sample8_b.yaml new file mode 100644 index 0000000..87ae47c --- /dev/null +++ b/pkg/diff/testdata/sample8_b.yaml @@ -0,0 +1,65 @@ +Mode: structured +ContextAttributes: + specversion: 1.0 + type: com.example.someevent + time: 2018-04-05T03:56:14Z + id: 4321-4321-4321-a + source: /mycontext/subcontext + Extensions: + comexampleextension1 : "this one has no subject" + comexampleextension2 : | + {"othervalue": 5} +TransportExtensions: + user-agent: "foo" +Data: | + {"world":"hello"} +--- +Mode: structured +ContextAttributes: + specversion: 1.0 + type: com.example.someevent + time: 2018-04-05T03:56:14Z + id: 4321-4321-4321-b + source: /mycontext/subcontext + subject: first-subject + Extensions: + comexampleextension1 : "value" + comexampleextension2 : | + {"othervalue": 5} +TransportExtensions: + user-agent: "foo" +Data: | + {"world":"hello"} +--- +Mode: structured +ContextAttributes: + specversion: 1.0 + type: com.example.someevent + time: 2018-04-05T03:56:24Z + id: 4321-4321-4321-c + subject: second-subject + Extensions: + comexampleextension1 : "this one has no subject" + comexampleextension2 : | + {"othervalue": 5} +TransportExtensions: + user-agent: "foo" +Data: | + {"world":"hello"} +--- +Mode: structured +ContextAttributes: + specversion: 1.0 + type: com.example.someevent + time: 2018-04-05T03:56:24Z + id: 4321-4321-4321-d + source: /mycontext/subcontext + subject: second-subject + Extensions: + comexampleextension1 : "value" + comexampleextension2 : | + {"othervalue": 5} +TransportExtensions: + user-agent: "foo" +Data: | + {"world":"hello"}