From 14d4d7be7aa3d10d947de6722846fc248fc45afa Mon Sep 17 00:00:00 2001 From: Jake Coffman Date: Wed, 16 Nov 2022 16:57:12 -0600 Subject: [PATCH] improve result diffing (#40) --- go.mod | 3 +- go.sum | 5 - internal/model/update.go | 5 + internal/server/api.go | 176 ++++++++++++++++++++++-------------- internal/server/api_test.go | 14 +++ 5 files changed, 127 insertions(+), 76 deletions(-) create mode 100644 internal/server/api_test.go diff --git a/go.mod b/go.mod index 0a4e7cb..4aac648 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/docker/docker v20.10.21+incompatible github.com/moby/moby v20.10.21+incompatible github.com/moby/sys/signal v0.7.0 - github.com/sergi/go-diff v1.2.0 github.com/spf13/cobra v1.6.1 gopkg.in/yaml.v3 v3.0.1 ) @@ -22,6 +21,7 @@ require ( github.com/gogo/protobuf v1.3.2 // indirect github.com/google/go-cmp v0.5.5 // indirect github.com/inconshreveable/mousetrap v1.0.1 // indirect + github.com/kr/pretty v0.1.0 // indirect github.com/moby/term v0.0.0-20220808134915-39b0c02b01ae // indirect github.com/morikuni/aec v1.0.0 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect @@ -33,5 +33,6 @@ require ( golang.org/x/net v0.0.0-20201021035429-f5854403a974 // indirect golang.org/x/sys v0.1.0 // indirect golang.org/x/time v0.0.0-20220609170525-579cf78fd858 // indirect + gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect gotest.tools/v3 v3.3.0 // indirect ) diff --git a/go.sum b/go.sum index 8fc506e..a4213f7 100644 --- a/go.sum +++ b/go.sum @@ -53,8 +53,6 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ= -github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= github.com/sirupsen/logrus v1.7.0 h1:ShrD1U9pZB12TX0cVy0DtePoCH97K8EtX+mg7ZARUtM= github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= github.com/spf13/cobra v1.6.1 h1:o94oiPyS4KD1mPy2fmcYYHHfCxLqYjJOhGsCHFZtEzA= @@ -65,7 +63,6 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= -github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= @@ -112,8 +109,6 @@ golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8T gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/model/update.go b/internal/model/update.go index 4d39d27..4935a70 100644 --- a/internal/model/update.go +++ b/internal/model/update.go @@ -51,3 +51,8 @@ type RecordPackageManagerVersion struct { Ecosystem string `json:"ecosystem" yaml:"ecosystem"` PackageManagers map[string]any `json:"package-managers" yaml:"package-managers"` } + +type RecordUpdateJobError struct { + ErrorType string `json:"error-type" yaml:"error-type"` + ErrorDetails map[string]any `json:"error-details" yaml:"error-details"` +} diff --git a/internal/server/api.go b/internal/server/api.go index ad609e8..c051021 100644 --- a/internal/server/api.go +++ b/internal/server/api.go @@ -11,12 +11,11 @@ import ( "net" "net/http" "os" + "reflect" "runtime" "strings" "time" - "github.com/sergi/go-diff/diffmatchpatch" - "github.com/dependabot/cli/internal/model" "gopkg.in/yaml.v3" ) @@ -110,8 +109,12 @@ func (a *API) ServeHTTP(_ http.ResponseWriter, r *http.Request) { parts := strings.Split(r.URL.String(), "/") kind := parts[len(parts)-1] + actual, err := decodeWrapper(kind, data) + if err != nil { + a.pushError(err) + } - if err := a.pushResult(kind, data); err != nil { + if err := a.pushResult(kind, actual); err != nil { a.pushError(err) return } @@ -120,10 +123,10 @@ func (a *API) ServeHTTP(_ http.ResponseWriter, r *http.Request) { return } - a.assertExpectation(kind, data) + a.assertExpectation(kind, actual) } -func (a *API) assertExpectation(kind string, actualData []byte) { +func (a *API) assertExpectation(kind string, actual *model.UpdateWrapper) { if len(a.Expectations) <= a.cursor { err := fmt.Errorf("missing expectation") a.pushError(err) @@ -134,24 +137,19 @@ func (a *API) assertExpectation(kind string, actualData []byte) { if kind != expect.Type { err := fmt.Errorf("type was unexpected: expected %v got %v", expect.Type, kind) a.pushError(err) + return } - expectJSON, _ := json.Marshal(expect.Expect) - // pretty both for sorting and can use simple comparison - prettyData, _ := pretty(string(expectJSON)) - actual, _ := pretty(string(actualData)) - if actual != prettyData { - err := fmt.Errorf("expected output doesn't match actual data received") + // need to use decodeWrapper to get the right type to match the actual type + data, err := json.Marshal(expect.Expect) + if err != nil { + panic(err) + } + expected, err := decodeWrapper(expect.Type, data) + if err != nil { + panic(err) + } + if err = compare(expected, actual); err != nil { a.pushError(err) - - // print diff to stdout - dmp := diffmatchpatch.New() - - const checklines = false - diffs := dmp.DiffMain(prettyData, actual, checklines) - - diffs = dmp.DiffCleanupSemantic(diffs) - - fmt.Println(dmp.DiffPrettyText(diffs)) } } @@ -162,11 +160,7 @@ func (a *API) pushError(err error) { a.Errors = append(a.Errors, err) } -func (a *API) pushResult(kind string, data []byte) error { - actual, err := decodeWrapper(kind, data) - if err != nil { - return err - } +func (a *API) pushResult(kind string, actual *model.UpdateWrapper) error { // TODO validate required data output := model.Output{ Type: kind, @@ -182,66 +176,108 @@ func (a *API) pushResult(kind string, data []byte) error { return nil } -// pretty indents and sorts the keys for a consistent comparison -func pretty(jsonString string) (string, error) { - var v map[string]any - if err := json.Unmarshal([]byte(jsonString), &v); err != nil { - return "", err - } - removeNullsFromObjects(v) - // shouldn't be possible to error - b, _ := json.MarshalIndent(v, "", " ") - return string(b), nil -} - -func removeNullsFromObjects(m map[string]any) { - for k, v := range m { - switch assertedVal := v.(type) { - case nil: - delete(m, k) - case map[string]any: - removeNullsFromObjects(assertedVal) - case []any: - for _, item := range assertedVal { - switch assertedItem := item.(type) { - case map[string]any: - removeNullsFromObjects(assertedItem) - } - } - } - } -} - -func decodeWrapper(kind string, data []byte) (*model.UpdateWrapper, error) { - var actual model.UpdateWrapper +func decodeWrapper(kind string, data []byte) (actual *model.UpdateWrapper, err error) { + actual = &model.UpdateWrapper{} switch kind { case "update_dependency_list": - actual.Data = decode[model.UpdateDependencyList](data) + actual.Data, err = decode[model.UpdateDependencyList](data) case "create_pull_request": - actual.Data = decode[model.CreatePullRequest](data) + actual.Data, err = decode[model.CreatePullRequest](data) case "update_pull_request": - actual.Data = decode[model.UpdatePullRequest](data) + actual.Data, err = decode[model.UpdatePullRequest](data) case "close_pull_request": - actual.Data = decode[model.ClosePullRequest](data) + actual.Data, err = decode[model.ClosePullRequest](data) case "mark_as_processed": - actual.Data = decode[model.MarkAsProcessed](data) + actual.Data, err = decode[model.MarkAsProcessed](data) case "record_package_manager_version": - actual.Data = decode[model.RecordPackageManagerVersion](data) + actual.Data, err = decode[model.RecordPackageManagerVersion](data) case "record_update_job_error": - actual.Data = decode[map[string]any](data) + actual.Data, err = decode[model.RecordUpdateJobError](data) default: return nil, fmt.Errorf("unexpected output type: %s", kind) } - return &actual, nil + return actual, err } -func decode[T any](data []byte) any { +func decode[T any](data []byte) (any, error) { var wrapper struct { Data T `json:"data" yaml:"data"` } - err := yaml.NewDecoder(bytes.NewBuffer(data)).Decode(&wrapper) + decoder := yaml.NewDecoder(bytes.NewBuffer(data)) + decoder.KnownFields(true) + err := decoder.Decode(&wrapper) if err != nil { - panic(err) + return nil, err + } + return wrapper.Data, nil +} + +func compare(expect, actual *model.UpdateWrapper) error { + switch v := expect.Data.(type) { + case model.UpdateDependencyList: + return compareUpdateDependencyList(v, actual.Data.(model.UpdateDependencyList)) + case model.CreatePullRequest: + return compareCreatePullRequest(v, actual.Data.(model.CreatePullRequest)) + case model.UpdatePullRequest: + return compareUpdatePullRequest(v, actual.Data.(model.UpdatePullRequest)) + case model.ClosePullRequest: + return compareClosePullRequest(v, actual.Data.(model.ClosePullRequest)) + case model.RecordPackageManagerVersion: + return compareRecordPackageManagerVersion(v, actual.Data.(model.RecordPackageManagerVersion)) + case model.MarkAsProcessed: + return compareMarkAsProcessed(v, actual.Data.(model.MarkAsProcessed)) + case model.RecordUpdateJobError: + return compareRecordUpdateJobError(v, actual.Data.(model.RecordUpdateJobError)) + default: + return fmt.Errorf("unexpected type: %s", reflect.TypeOf(v)) + } +} + +func compareUpdateDependencyList(expect, actual model.UpdateDependencyList) error { + if reflect.DeepEqual(expect, actual) { + return nil + } + return fmt.Errorf("dependency list was unexpected") +} + +func compareCreatePullRequest(expect, actual model.CreatePullRequest) error { + if reflect.DeepEqual(expect, actual) { + return nil + } + return fmt.Errorf("create pull request was unexpected") +} + +func compareUpdatePullRequest(expect, actual model.UpdatePullRequest) error { + if reflect.DeepEqual(expect, actual) { + return nil + } + return fmt.Errorf("update pull request was unexpected") +} + +func compareClosePullRequest(expect, actual model.ClosePullRequest) error { + if reflect.DeepEqual(expect, actual) { + return nil + } + return fmt.Errorf("close pull request was unexpected") +} + +func compareRecordPackageManagerVersion(expect, actual model.RecordPackageManagerVersion) error { + if reflect.DeepEqual(expect, actual) { + return nil + } + return fmt.Errorf("record package manager version was unexpected") +} + +func compareMarkAsProcessed(expect, actual model.MarkAsProcessed) error { + if reflect.DeepEqual(expect, actual) { + return nil + } + return fmt.Errorf("mark as processed was unexpected") +} + +func compareRecordUpdateJobError(expect, actual model.RecordUpdateJobError) error { + if reflect.DeepEqual(expect, actual) { + return nil } - return wrapper.Data + return fmt.Errorf("record update job error was unexpected") } diff --git a/internal/server/api_test.go b/internal/server/api_test.go new file mode 100644 index 0000000..c3b2b27 --- /dev/null +++ b/internal/server/api_test.go @@ -0,0 +1,14 @@ +package server + +import ( + "testing" +) + +func Test_decodeWrapper(t *testing.T) { + t.Run("reject extra data", func(t *testing.T) { + _, err := decodeWrapper("update_dependency_list", []byte(`data: {"unknown": "value"}`)) + if err == nil { + t.Error("expected decode would error on extra data") + } + }) +}