internal/testrunner/runners/pipeline: replace jsondiff output with difflib #872
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
🌐 Coverage report
|
…fflib This gives a correct output for arrays and provides meaningful context for finding issues in test-case source.
|
|
||
| // Configure diff. | ||
| opts.SkipMatches = true | ||
| opts.CompareNumbers = compareJsonNumbers |
There was a problem hiding this comment.
compareJsonNumbers and its test can be removed if we stop using jsondiff.
There was a problem hiding this comment.
I was originally thinking that, but the logic behind using it made a lot of sense, to I used it as the Comparer for cmp.Equal.
There was a problem hiding this comment.
Ah, you are right, I didn't see that it was still used there 👍
| return "", nil | ||
| } | ||
|
|
||
| result, diff := jsondiff.Compare(want, got, &opts) |
There was a problem hiding this comment.
We could keep this comparison method even if later we change the diff that we print, right?
| return "", err | ||
| } | ||
| return diff, nil | ||
| want, err = marshalNormalizedJSON(wantVal) |
There was a problem hiding this comment.
marshalNormalizedJSON marshals and unmarshals using jsonUnmarshalUsingNumber, something that has been already done here. We could do the final marshal directly here instead of repeating the operations.
Not so important though as this code is only used if the documents are different.
There was a problem hiding this comment.
I was being conservative to ensure that the values were being treated the same way and to ensure that the reader was clear that this was happening.
|
|
||
| // Configure diff. | ||
| opts.SkipMatches = true | ||
| opts.CompareNumbers = compareJsonNumbers |
There was a problem hiding this comment.
Ah, you are right, I didn't see that it was still used there 👍
3 participants
This gives a correct output for arrays and provides meaningful context for finding issues in test-case source. (Note also that this is the library that is used in beats for diff output).
Please take a look.