Skip to content

Better diffs in tests #16112

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

Merged
merged 5 commits into from
Sep 15, 2023
Merged

Better diffs in tests #16112

merged 5 commits into from
Sep 15, 2023

Conversation

hauntsaninja
Copy link
Collaborator

It's annoying that one line change causes everything else to show up as a diff. Just use difflib instead. I also highlight the changed lines. We can't use FancyFormatter because it doesn't work well with pytest.

hauntsaninja and others added 2 commits September 14, 2023 02:00
It's annoying that one line change causes everything else to show up as
a diff. Just use difflib instead. I also highlight the changed lines.
We can't use FancyFormatter because it doesn't work well with pytest.
@ikonst
Copy link
Contributor

ikonst commented Sep 14, 2023

Add a test to mypy/test/meta/?

@JukkaL
Copy link
Collaborator

JukkaL commented Sep 14, 2023

Can you show some examples?

One the goals of the existing diffing approach is to make it easy to copy-paste the actual output from running a test to a test case description with only minor editing required. I'd like to preserve that. I often leave the output of a test case empty, run the test case, and after validating that the output looks good copy the output into the test case.

@ikonst
Copy link
Contributor

ikonst commented Sep 14, 2023

@JukkaL did you try pytest --update-data recently? :)

@JukkaL
Copy link
Collaborator

JukkaL commented Sep 14, 2023

did you try pytest --update-data recently?

It's pretty handy, but I only tend to use it for bulk updates. If I'm working on a single test case I tend to use copy-paste a lot, in part because I can never remember the name of the flag without looking it up. :)

@hauntsaninja
Copy link
Collaborator Author

Before:
Screenshot 2023-09-14 at 11 27 38 AM
After:
Screenshot 2023-09-14 at 11 27 46 AM

@hauntsaninja
Copy link
Collaborator Author

Okay I changed it to also mention --update-data (which is awesome)

@ikonst
Copy link
Contributor

ikonst commented Sep 14, 2023

Speaking of --update-data: A wart most people would quickly discover is that (a) we default to parallelized tests and (b) those are incompatible with --update-data (since a file might be sharded between two runners, and you can't update files in parallel while relying on absolute line offsets).

I couldn't find a way to trigger -n0 when --update-data is used.

We can rewrite --update-data to work by keeping offsets relative to testcase, which would require it to at least rudimentarily parse the file during the update phase.

Or keep the wart (maybe then say "Update the test output using -n0 --update-data").

@hauntsaninja
Copy link
Collaborator Author

hauntsaninja commented Sep 15, 2023

I'm merging, since I keep wanting this for other PRs I'm working on. I will add a unit test for the diff logic + any other feedback in follow up. If it helps I use similar code elsewhere + this logic never changes test results, so am pretty confident

@hauntsaninja hauntsaninja merged commit d77310a into python:master Sep 15, 2023
@hauntsaninja hauntsaninja deleted the better-diff branch September 15, 2023 07:17
hauntsaninja added a commit to hauntsaninja/mypy that referenced this pull request Oct 1, 2023
hauntsaninja added a commit to hauntsaninja/mypy that referenced this pull request Oct 1, 2023
hauntsaninja added a commit that referenced this pull request Oct 3, 2023
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