Skip to content

Commit 14d4d7b

Browse files
authored
improve result diffing (#40)
1 parent 3850ad2 commit 14d4d7b

5 files changed

Lines changed: 127 additions & 76 deletions

File tree

‎go.mod‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ require (
88
github.com/docker/docker v20.10.21+incompatible
99
github.com/moby/moby v20.10.21+incompatible
1010
github.com/moby/sys/signal v0.7.0
11-
github.com/sergi/go-diff v1.2.0
1211
github.com/spf13/cobra v1.6.1
1312
gopkg.in/yaml.v3 v3.0.1
1413
)
@@ -22,6 +21,7 @@ require (
2221
github.com/gogo/protobuf v1.3.2 // indirect
2322
github.com/google/go-cmp v0.5.5 // indirect
2423
github.com/inconshreveable/mousetrap v1.0.1 // indirect
24+
github.com/kr/pretty v0.1.0 // indirect
2525
github.com/moby/term v0.0.0-20220808134915-39b0c02b01ae // indirect
2626
github.com/morikuni/aec v1.0.0 // indirect
2727
github.com/opencontainers/go-digest v1.0.0 // indirect
@@ -33,5 +33,6 @@ require (
3333
golang.org/x/net v0.0.0-20201021035429-f5854403a974 // indirect
3434
golang.org/x/sys v0.1.0 // indirect
3535
golang.org/x/time v0.0.0-20220609170525-579cf78fd858 // indirect
36+
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
3637
gotest.tools/v3 v3.3.0 // indirect
3738
)

‎go.sum‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE
5353
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
5454
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
5555
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
56-
github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ=
57-
github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
5856
github.com/sirupsen/logrus v1.7.0 h1:ShrD1U9pZB12TX0cVy0DtePoCH97K8EtX+mg7ZARUtM=
5957
github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
6058
github.com/spf13/cobra v1.6.1 h1:o94oiPyS4KD1mPy2fmcYYHHfCxLqYjJOhGsCHFZtEzA=
@@ -65,7 +63,6 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An
6563
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
6664
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
6765
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
68-
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
6966
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
7067
github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
7168
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
112109
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
113110
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
114111
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
115-
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
116-
gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
117112
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
118113
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
119114
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

‎internal/model/update.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,8 @@ type RecordPackageManagerVersion struct {
5151
Ecosystem string `json:"ecosystem" yaml:"ecosystem"`
5252
PackageManagers map[string]any `json:"package-managers" yaml:"package-managers"`
5353
}
54+
55+
type RecordUpdateJobError struct {
56+
ErrorType string `json:"error-type" yaml:"error-type"`
57+
ErrorDetails map[string]any `json:"error-details" yaml:"error-details"`
58+
}

‎internal/server/api.go‎

Lines changed: 106 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,11 @@ import (
1111
"net"
1212
"net/http"
1313
"os"
14+
"reflect"
1415
"runtime"
1516
"strings"
1617
"time"
1718

18-
"github.com/sergi/go-diff/diffmatchpatch"
19-
2019
"github.com/dependabot/cli/internal/model"
2120
"gopkg.in/yaml.v3"
2221
)
@@ -110,8 +109,12 @@ func (a *API) ServeHTTP(_ http.ResponseWriter, r *http.Request) {
110109

111110
parts := strings.Split(r.URL.String(), "/")
112111
kind := parts[len(parts)-1]
112+
actual, err := decodeWrapper(kind, data)
113+
if err != nil {
114+
a.pushError(err)
115+
}
113116

114-
if err := a.pushResult(kind, data); err != nil {
117+
if err := a.pushResult(kind, actual); err != nil {
115118
a.pushError(err)
116119
return
117120
}
@@ -120,10 +123,10 @@ func (a *API) ServeHTTP(_ http.ResponseWriter, r *http.Request) {
120123
return
121124
}
122125

123-
a.assertExpectation(kind, data)
126+
a.assertExpectation(kind, actual)
124127
}
125128

126-
func (a *API) assertExpectation(kind string, actualData []byte) {
129+
func (a *API) assertExpectation(kind string, actual *model.UpdateWrapper) {
127130
if len(a.Expectations) <= a.cursor {
128131
err := fmt.Errorf("missing expectation")
129132
a.pushError(err)
@@ -134,24 +137,19 @@ func (a *API) assertExpectation(kind string, actualData []byte) {
134137
if kind != expect.Type {
135138
err := fmt.Errorf("type was unexpected: expected %v got %v", expect.Type, kind)
136139
a.pushError(err)
140+
return
137141
}
138-
expectJSON, _ := json.Marshal(expect.Expect)
139-
// pretty both for sorting and can use simple comparison
140-
prettyData, _ := pretty(string(expectJSON))
141-
actual, _ := pretty(string(actualData))
142-
if actual != prettyData {
143-
err := fmt.Errorf("expected output doesn't match actual data received")
142+
// need to use decodeWrapper to get the right type to match the actual type
143+
data, err := json.Marshal(expect.Expect)
144+
if err != nil {
145+
panic(err)
146+
}
147+
expected, err := decodeWrapper(expect.Type, data)
148+
if err != nil {
149+
panic(err)
150+
}
151+
if err = compare(expected, actual); err != nil {
144152
a.pushError(err)
145-
146-
// print diff to stdout
147-
dmp := diffmatchpatch.New()
148-
149-
const checklines = false
150-
diffs := dmp.DiffMain(prettyData, actual, checklines)
151-
152-
diffs = dmp.DiffCleanupSemantic(diffs)
153-
154-
fmt.Println(dmp.DiffPrettyText(diffs))
155153
}
156154
}
157155

@@ -162,11 +160,7 @@ func (a *API) pushError(err error) {
162160
a.Errors = append(a.Errors, err)
163161
}
164162

165-
func (a *API) pushResult(kind string, data []byte) error {
166-
actual, err := decodeWrapper(kind, data)
167-
if err != nil {
168-
return err
169-
}
163+
func (a *API) pushResult(kind string, actual *model.UpdateWrapper) error {
170164
// TODO validate required data
171165
output := model.Output{
172166
Type: kind,
@@ -182,66 +176,108 @@ func (a *API) pushResult(kind string, data []byte) error {
182176
return nil
183177
}
184178

185-
// pretty indents and sorts the keys for a consistent comparison
186-
func pretty(jsonString string) (string, error) {
187-
var v map[string]any
188-
if err := json.Unmarshal([]byte(jsonString), &v); err != nil {
189-
return "", err
190-
}
191-
removeNullsFromObjects(v)
192-
// shouldn't be possible to error
193-
b, _ := json.MarshalIndent(v, "", " ")
194-
return string(b), nil
195-
}
196-
197-
func removeNullsFromObjects(m map[string]any) {
198-
for k, v := range m {
199-
switch assertedVal := v.(type) {
200-
case nil:
201-
delete(m, k)
202-
case map[string]any:
203-
removeNullsFromObjects(assertedVal)
204-
case []any:
205-
for _, item := range assertedVal {
206-
switch assertedItem := item.(type) {
207-
case map[string]any:
208-
removeNullsFromObjects(assertedItem)
209-
}
210-
}
211-
}
212-
}
213-
}
214-
215-
func decodeWrapper(kind string, data []byte) (*model.UpdateWrapper, error) {
216-
var actual model.UpdateWrapper
179+
func decodeWrapper(kind string, data []byte) (actual *model.UpdateWrapper, err error) {
180+
actual = &model.UpdateWrapper{}
217181
switch kind {
218182
case "update_dependency_list":
219-
actual.Data = decode[model.UpdateDependencyList](data)
183+
actual.Data, err = decode[model.UpdateDependencyList](data)
220184
case "create_pull_request":
221-
actual.Data = decode[model.CreatePullRequest](data)
185+
actual.Data, err = decode[model.CreatePullRequest](data)
222186
case "update_pull_request":
223-
actual.Data = decode[model.UpdatePullRequest](data)
187+
actual.Data, err = decode[model.UpdatePullRequest](data)
224188
case "close_pull_request":
225-
actual.Data = decode[model.ClosePullRequest](data)
189+
actual.Data, err = decode[model.ClosePullRequest](data)
226190
case "mark_as_processed":
227-
actual.Data = decode[model.MarkAsProcessed](data)
191+
actual.Data, err = decode[model.MarkAsProcessed](data)
228192
case "record_package_manager_version":
229-
actual.Data = decode[model.RecordPackageManagerVersion](data)
193+
actual.Data, err = decode[model.RecordPackageManagerVersion](data)
230194
case "record_update_job_error":
231-
actual.Data = decode[map[string]any](data)
195+
actual.Data, err = decode[model.RecordUpdateJobError](data)
232196
default:
233197
return nil, fmt.Errorf("unexpected output type: %s", kind)
234198
}
235-
return &actual, nil
199+
return actual, err
236200
}
237201

238-
func decode[T any](data []byte) any {
202+
func decode[T any](data []byte) (any, error) {
239203
var wrapper struct {
240204
Data T `json:"data" yaml:"data"`
241205
}
242-
err := yaml.NewDecoder(bytes.NewBuffer(data)).Decode(&wrapper)
206+
decoder := yaml.NewDecoder(bytes.NewBuffer(data))
207+
decoder.KnownFields(true)
208+
err := decoder.Decode(&wrapper)
243209
if err != nil {
244-
panic(err)
210+
return nil, err
211+
}
212+
return wrapper.Data, nil
213+
}
214+
215+
func compare(expect, actual *model.UpdateWrapper) error {
216+
switch v := expect.Data.(type) {
217+
case model.UpdateDependencyList:
218+
return compareUpdateDependencyList(v, actual.Data.(model.UpdateDependencyList))
219+
case model.CreatePullRequest:
220+
return compareCreatePullRequest(v, actual.Data.(model.CreatePullRequest))
221+
case model.UpdatePullRequest:
222+
return compareUpdatePullRequest(v, actual.Data.(model.UpdatePullRequest))
223+
case model.ClosePullRequest:
224+
return compareClosePullRequest(v, actual.Data.(model.ClosePullRequest))
225+
case model.RecordPackageManagerVersion:
226+
return compareRecordPackageManagerVersion(v, actual.Data.(model.RecordPackageManagerVersion))
227+
case model.MarkAsProcessed:
228+
return compareMarkAsProcessed(v, actual.Data.(model.MarkAsProcessed))
229+
case model.RecordUpdateJobError:
230+
return compareRecordUpdateJobError(v, actual.Data.(model.RecordUpdateJobError))
231+
default:
232+
return fmt.Errorf("unexpected type: %s", reflect.TypeOf(v))
233+
}
234+
}
235+
236+
func compareUpdateDependencyList(expect, actual model.UpdateDependencyList) error {
237+
if reflect.DeepEqual(expect, actual) {
238+
return nil
239+
}
240+
return fmt.Errorf("dependency list was unexpected")
241+
}
242+
243+
func compareCreatePullRequest(expect, actual model.CreatePullRequest) error {
244+
if reflect.DeepEqual(expect, actual) {
245+
return nil
246+
}
247+
return fmt.Errorf("create pull request was unexpected")
248+
}
249+
250+
func compareUpdatePullRequest(expect, actual model.UpdatePullRequest) error {
251+
if reflect.DeepEqual(expect, actual) {
252+
return nil
253+
}
254+
return fmt.Errorf("update pull request was unexpected")
255+
}
256+
257+
func compareClosePullRequest(expect, actual model.ClosePullRequest) error {
258+
if reflect.DeepEqual(expect, actual) {
259+
return nil
260+
}
261+
return fmt.Errorf("close pull request was unexpected")
262+
}
263+
264+
func compareRecordPackageManagerVersion(expect, actual model.RecordPackageManagerVersion) error {
265+
if reflect.DeepEqual(expect, actual) {
266+
return nil
267+
}
268+
return fmt.Errorf("record package manager version was unexpected")
269+
}
270+
271+
func compareMarkAsProcessed(expect, actual model.MarkAsProcessed) error {
272+
if reflect.DeepEqual(expect, actual) {
273+
return nil
274+
}
275+
return fmt.Errorf("mark as processed was unexpected")
276+
}
277+
278+
func compareRecordUpdateJobError(expect, actual model.RecordUpdateJobError) error {
279+
if reflect.DeepEqual(expect, actual) {
280+
return nil
245281
}
246-
return wrapper.Data
282+
return fmt.Errorf("record update job error was unexpected")
247283
}

‎internal/server/api_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package server
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func Test_decodeWrapper(t *testing.T) {
8+
t.Run("reject extra data", func(t *testing.T) {
9+
_, err := decodeWrapper("update_dependency_list", []byte(`data: {"unknown": "value"}`))
10+
if err == nil {
11+
t.Error("expected decode would error on extra data")
12+
}
13+
})
14+
}

0 commit comments

Comments
 (0)