Skip to content
New issue

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

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

Already on GitHub? # to your account

use insta for witness tests #935

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

suaviloquence
Copy link
Contributor

See also #933 (comment).

Replaces custom snapshot code with insta when testing witness generation for lints, which should have better contributor experience.

@suaviloquence
Copy link
Contributor Author

It's still ron, so multiline strings will not be rendered super well. We could use a different serializer like yaml if this becomes a problem, and it would be very easy to change.

@obi1kenobi
Copy link
Owner

yaml is an interesting idea. If you try it and think it looks good, do you mind switching this to it?

I suspect many of our witnesses will be multi-line once we get past the basic "item deleted" ones, and diffing multi-line strings represented with escape sequences is going to get really annoying quickly. We risk letting bugs slip just because it was hard to notice the snapshot changed in an undesirable way (e.g. bad indentation etc.)

@suaviloquence
Copy link
Contributor Author

It looks like the yaml serializer that insta uses doesn't support multiline strings (even though yaml-the-langauge does...) mitsuhiko/insta#372. However toml handles multiline strings perfectly, my test code is below (i added extra lines to the witness hint)

---
source: src/query.rs
description: "Lint `function_missing` did not have the expected witness output.\nSee https://github.com/obi1kenobi/cargo-semver-checks/blob/main/CONTRIBUTING.md#testing-witnesses\nfor more information."
expression: "&actual_witnesses"
---
[["./test_crates/feature_flags_validation/"]]
filename = 'src/lib.rs'
begin_line = 4
hint = '''
line 1
line 2
feature_flags_validation::foo_becomes_gated(...);'''

[["./test_crates/feature_flags_validation/"]]
filename = 'src/lib.rs'
begin_line = 5
hint = '''
line 1
line 2
feature_flags_validation::bar_becomes_gated(...);'''

[["./test_crates/features_simple/"]]
filename = 'src/lib.rs'
begin_line = 2
hint = '''
line 1
line 2
features_simple::feature_dependent_function(...);'''

[["./test_crates/function_const_removed/"]]
filename = 'src/lib.rs'
begin_line = 5
hint = '''
line 1
line 2
function_const_removed::fn_removed(...);'''

[["./test_crates/function_feature_changed/"]]
filename = 'src/lib.rs'
begin_line = 2
hint = '''
line 1
line 2
function_feature_changed::moving_from_feature_A_to_feature_B(...);'''

[["./test_crates/function_missing/"]]
filename = 'src/lib.rs'
begin_line = 1
hint = '''
line 1
line 2
function_missing::will_be_removed_fn(...);'''

[["./test_crates/function_missing/"]]
filename = 'src/lib.rs'
begin_line = 4
hint = '''
line 1
line 2
function_missing::pub_use_removed_fn(...);'''

Copy link
Owner

@obi1kenobi obi1kenobi left a comment

Choose a reason for hiding this comment

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

Awesome work! 🚀

@obi1kenobi obi1kenobi enabled auto-merge (squash) September 19, 2024 00:07
@obi1kenobi obi1kenobi merged commit 1bd25e6 into obi1kenobi:main Sep 19, 2024
32 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants