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

Test snapshots with inline checks #681

Closed
wants to merge 4 commits into from

Conversation

squiddy
Copy link
Contributor

@squiddy squiddy commented Nov 11, 2022

This is a massive diff because I'm introducing new snapshots. The first and third commit are the relevant ones. To motivate this a bit:

When developing new lints or checking existing lints, I did one of these two things:

  • manually "translated" the location from the yaml snapshot and check against the test fixture
  • run cargo against the test fixtures

I also noticed that some test fixtures have inline comments/decorators to clarify what is expected.


What I'm doing here is to add a new snapshot (to potentially replace the existing one eventually) that inlines the checks into the source code, similar to what we'd expect with #525 (which I've been looking into, hence this first step).
Looking at these snapshots it's easier to visually confirm the correct location (currently just row based though).

What I did notice is that in some cases the end location is pretty far off, due to whitespace following it. The third commit is an attempt to narrow down this location to have a closer match.

Example initial snapshot

carbon

Example snapshot with narrowed down location

carbon2

Idea here is to make it easier to visually confirm that the checks are
in the right location. When developing lints, I had to "manually"
transfer the location (row, column) from the existing snapshot to the
original source file fixture.

This also serves as a first step toward better reporting with source
code.
Move up the end location of the check to the nearest non-empty line.
This removes trailing empty lines from the range of code the check is
reporting.
@squiddy
Copy link
Contributor Author

squiddy commented Nov 11, 2022

For narrowing down I need to check whether this is actually necessary or, in case the end points to the start of a line, I'll just report the end of the check for the line before. Might just be being confused by the locations.

@squiddy squiddy closed this Nov 12, 2022
@squiddy squiddy deleted the test-snapshots branch November 12, 2022 13:00
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.

1 participant