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

New function with simpler API #62

Open
alexmojaki opened this issue Apr 13, 2024 · 10 comments
Open

New function with simpler API #62

alexmojaki opened this issue Apr 13, 2024 · 10 comments

Comments

@alexmojaki
Copy link

alexmojaki commented Apr 13, 2024

Here's a proposal for what I think my ideal API/workflow would look like. Hopefully if you don't like all of it you can still extract some good ideas.

from inline_snapshot import snap

def test_foo():
    # Instead of:
    # assert foo() == snapshot(bar)
    snap(foo(), bar)

    # Instead of:
    # assert baz() == snapshot()
    snap(baz())

When the second argument doesn't exist or doesn't equal the first:

  • The argument is created/updated without needing --inline-snapshot=fix/create/update or any other configuration, it's just the default behaviour.
  • An error is raised so that the test fails. The error message contains a diff of pretty formatted values in the same way as pytest.

Reasoning:

  1. A new function can have drastically different behaviour without disrupting existing usage.
  2. Having a function that doesn't need to be in an assert means that executing works fine without needing to disable pytest rewriting, fulfilling the desire in Allow fixing snapshots without disabling pytest rewriting #57.
  3. No configuration or knowledge required. It just works, even for newcomers to a codebase who have never seen inline_snapshot.
  4. No risk of tests passing when they shouldn't.
  5. It's immediately obvious when a snapshot didn't match (by seeing the test failures) without having to look at the test file, which is great when running many tests that need fixing.
  6. If I run all tests and several snapshots in different places get updated, I can easily use existing pytest/PyCharm features to re-run just the failed tests to update those snapshots again without running the other tests or having to gather a list of specific tests. This would have been especially great in recent usage.
  7. I can run a single test in PyCharm with two clicks and also update its snapshot if needed, without needing to type anything. Right now the way that I avoid typing --inline-snapshot=fix is by associating that with whole files where I expect to reuse that configuration, but it becomes less practical for a single test function.
  8. It's significantly shorter and easier to type: assert , == and shot vs just , .
@15r10nk
Copy link
Owner

15r10nk commented Apr 13, 2024

No risk of tests passing when they shouldn't.

What do you mean here? Is there currently any risk that tests pass because of inline-snapshot?

I can run a single test in PyCharm with two clicks and also update its snapshot if needed.

"update if needed" ... I don't understand how you would do it in your proposal.
Snapshots should always be updated if I understand you correctly. How would you run pytest for one test without updating the snapshots?

I'm currently working on #63 which allows you to run pytest --inline-snapshot without the flags. It shows you a diff and asks you if you want to apply the changes. You can try it if you want and tell me if it solves some of your problems.

I have no experience with PyCharm, but I want to try it to see how it works with inline-snapshot. I hope this allows me to understand your workflow and your problems better.

@alexmojaki
Copy link
Author

No risk of tests passing when they shouldn't.

What do you mean here? Is there currently any risk that tests pass because of inline-snapshot?

Not currently, unless someone uses fix in a really stupid way. I mean this as a response to #57 (comment):

if --inline-snapshot=fix could be enabled by default.

This would mean that your snapshots are always fixed and your tests would always pass.

i.e. the proposal here would mean that snapshots are always fixed but tests don't always pass, so it's both convenient and safe.

I can run a single test in PyCharm with two clicks and also update its snapshot if needed.

"update if needed" ... I don't understand how you would do it in your proposal. Snapshots should always be updated if I understand you correctly. How would you run pytest for one test without updating the snapshots?

I just mean they wouldn't be updated if they were already correct.

I have no experience with PyCharm, but I want to try it to see how it works with inline-snapshot. I hope this allows me to understand your workflow and your problems better.

Here's what I mean by running a test with two clicks:

  1. Click the green button:

Screenshot from 2024-04-13 19-40-35

  1. Move the mouse a few pixels and click the first menu item.

Screenshot from 2024-04-13 19-40-52

This is way more convenient (and faster) than running pytest -k test_foo with or without --inline-snapshot=fix.

I basically want to still do just that and also see the test code update without further interaction.

I'm currently working on #63 which allows you to run pytest --inline-snapshot without the flags. It shows you a diff and asks you if you want to apply the changes. You can try it if you want and tell me if it solves some of your problems.

As I said in #48 (comment), I actually prefer to just apply the changes and review them after.

@15r10nk
Copy link
Owner

15r10nk commented Apr 13, 2024

i.e. the proposal here would mean that snapshots are always fixed but tests don't always pass, so it's both convenient and safe.

but the tests will pass in the next pytest run if they are fixed, right?

@alexmojaki
Copy link
Author

Right.

@15r10nk
Copy link
Owner

15r10nk commented Apr 13, 2024

And how would you debug test which fail because you broke something (where you don't want to change the snapshots)? would you use pytest with an argument like --inline-snapshot-disable in this case?

I fix my bugs more often then I change my snapshots and this solution would make my normal workflow more complicated.

@alexmojaki
Copy link
Author

I don't need to avoid changing the snapshot. A change in the snapshot is a visible change that's often easier to inspect than the diff from a test failure. If I didn't expect a change, then I fix the function and rerun the test until the change disappears. Basically I do the same thing I would do without inline-snapshot.

@15r10nk
Copy link
Owner

15r10nk commented Apr 13, 2024

Ahh, you are basically ignoring the pytest errors (almost) and rely more on the diff. changes in the diff mean you broke something.

I don't think that I would recommend this for everyone, but I think I could provide a config option which defines default "mode" which inline-snapshot uses. create,fix in your case.
I would not connect it to a special snap() function. It is more a workflow which is maybe different for every user and which should be consistent across the project.

I will think about it while I'm working on #63. Maybe there are more options which would be useful as a default.

You should be able to hack it into some branch and try it yourself for now.

How important is it for you that you have a snap() function?

@alexmojaki
Copy link
Author

It's not essential. An alternative way to achieve similar results would be to:

  1. Add a flag which makes Snapshot.__eq__ return False when appropriate so that tests fail.
  2. Solve Allow fixing snapshots without disabling pytest rewriting #57.

Then I would turn on that first flag and create,fix by default in pytest.ini.

@marklidenberg
Copy link

marklidenberg commented Apr 21, 2024

I agree that we definitely need a flag to raise an error if the expected value does not match the actual one. For me, the most intuitive default scenario for inline snapshots would be to use this flag along with inline-snapshot=create

In cases where only create flag is used, it seems sufficient to do the following:

@repr_wrapper
def snapshot(obj=undefined):
...
    if not _active:
        if obj is undefined:
            raise AssertionError(
                "your snapshot is missing a value run pytest with --inline-snapshot=create"
            )
        else:
            return 

# < --- new 
    if obj is not undefined and not _update_flags.update and not _update_flags.fix and not _update_flags.trim:
        return obj
# >
... 

@15r10nk
Copy link
Owner

15r10nk commented Apr 23, 2024

@alexmojaki

Add a flag which makes Snapshot.eq return False when appropriate so that tests fail.

something similar will become part of #63. No flag will affect the outcome of your tests.

Snapshot.__eq__ might however still return True if you use fix because it has to be able to fix multiple snapshots in the same test, but this failures will be recorded and be checked in a autouse fixture which will make your tests fail. Which will turn your assertions in soft-assertions in this case.

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

No branches or pull requests

3 participants