Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build plan snapshot testing #355

Merged
merged 2 commits into from
Jul 23, 2022
Merged

Conversation

coffee-cup
Copy link
Contributor

@coffee-cup coffee-cup commented Jul 22, 2022

What does this PR address?

Use cargo-insta for snapshot testing the build plan tests. This is useful since it will tests all parts of the build plan for each tests as opposed to just the parts that get manually asserted. This also makes updating the build plan format a lot easier since we don't have to go and edit a 900 line file everytime.

I am not snapshot testing and build plans that rely on ARCH. This brings up a larger question in that we should be able to build for other platforms in the tests and not rely on the ARCH constant. But that should be fixed in another PR.

@Milo123459
Copy link
Collaborator

I'm not sure, this kind of looks painful to maintain.

expression: plan
---
{
"version": "0.2.9",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have to input the version every time for each test? I presume we have to update it on every new version as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No these are never edited manually. They are used to compare the build plan output and make sure there are no changes, if there are changes you use cargo insta review to go through them one by one and either accept or reject the updates.

For example, after manually editing a .snap file and running

cargo insta test --review -- --test generate_plan_tests

I get

image

Pressing a will automatically update the snapshot with the changes

@coffee-cup
Copy link
Contributor Author

I'm not sure, this kind of looks painful to maintain.

Insta has tools to automatically review and update the snapshots. The snapshot files should never be edited manually.

This video explains how it works fairly well

https://www.youtube.com/watch?v=rCHrMqE4JOY

@coffee-cup coffee-cup merged commit 45d589d into main Jul 23, 2022
@coffee-cup coffee-cup deleted the jr/build-plan-snapshot-testing branch July 23, 2022 20:33
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.

None yet

3 participants