Skip to content

improve result diffing#40

Merged
jakecoffman merged 4 commits intomainfrom
jakecoffman/improve-diffing
Nov 16, 2022
Merged

improve result diffing#40
jakecoffman merged 4 commits intomainfrom
jakecoffman/improve-diffing

Conversation

@jakecoffman
Copy link
Member

@pavera is developing a smoke test for the Yarn Berry ecosystem which caches binaries by default.

This has rendered the diff view in the CLI useless, and actually a hinderance as I ran his test and the amount of output to stdout caused the updater to timeout!

Also the comparison of the expected and actual is slightly busted since it compares data we've unmarshalled into our model with the raw JSON coming in from a request. This has the result of some Go-isms (like default value vs null) being applied to one side and not the other.

Originally I did it this way because I was afraid our model was off, which we found in time that it was. Now that things are settling I have more confidence, but I've also added a line to the YAML decoder which will error if an unexpected value is encountered.

So this PR:

  1. Removes the diff output
  2. Compares the payload after being marshaled into our model

In the future I want to bring the diff output back but have it smart about what type it's diffing, which is why I broke out the different compare functions. I'll fill those in in the future!

@jakecoffman jakecoffman requested a review from a team as a code owner November 16, 2022 21:48
Copy link
Contributor

@pavera pavera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jakecoffman
Copy link
Member Author

I have really good timing, the bundler test is failing not because of this change: https://github.com/dependabot/smoke-tests/actions/runs/3483487230

@jakecoffman jakecoffman merged commit 14d4d7b into main Nov 16, 2022
@jakecoffman jakecoffman deleted the jakecoffman/improve-diffing branch November 16, 2022 22:57
@jeffwidman
Copy link
Member

Next time, can you squash your fixup commits like "silly lint" "missing error type" etc to keep the history cleaner?

@jakecoffman
Copy link
Member Author

They're squashed on main, I used the squash button.

@jeffwidman
Copy link
Member

Oh thanks, I totally overlooked that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants