Skip to content

Add a transformed_mir_diff expect test for #856 to diff against#867

Merged
rok-cesnovar merged 2 commits into
masterfrom
transformed-mir-expect-test
Apr 2, 2021
Merged

Add a transformed_mir_diff expect test for #856 to diff against#867
rok-cesnovar merged 2 commits into
masterfrom
transformed-mir-expect-test

Conversation

@seantalts
Copy link
Copy Markdown
Member

Just adding an expect test so we can track changes in the transformed MIR as well as the pre-transform MIR.

(no release notes)

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

@seantalts seantalts force-pushed the transformed-mir-expect-test branch from d309d01 to f5b2be2 Compare April 2, 2021 16:49
@seantalts seantalts force-pushed the transformed-mir-expect-test branch from f5b2be2 to b33c37d Compare April 2, 2021 16:50
Copy link
Copy Markdown
Member

@rok-cesnovar rok-cesnovar left a comment

Choose a reason for hiding this comment

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

Thanks Sean!

@seantalts
Copy link
Copy Markdown
Member Author

@rok-cesnovar while you're here, what do you think of this idea of adding the transformed MIR expect test as a diff from the pre-transform one? Is that too complicated? That's what's currently up - it's about 15000 lines as a diff, and about 30,000 lines not as a diff. Also, the diff logic uses bash so may not be portable - waiting for Jenkins to yell. I have the non-diff version ready to push if it doesn't work or you like the non-diff version better. I get a little confused thinking about future PRs comparing a diff of a diff...

@rok-cesnovar
Copy link
Copy Markdown
Member

Honestly, 30k is not that big and it makes is simpler to understand I guess? There are a bunch of distributions with 35k+ expected lines and those are not nearly as important as this.

@seantalts
Copy link
Copy Markdown
Member Author

Sounds good. Also, apparently diff has different behavior on mac, lol. Pushing up the simpler version, thank you!

@rok-cesnovar rok-cesnovar merged commit 036d930 into master Apr 2, 2021
@rok-cesnovar rok-cesnovar deleted the transformed-mir-expect-test branch April 2, 2021 19:53
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.

2 participants