15r10nk / inline-snapshot

create and update inline snapshots in your python tests
https://15r10nk.github.io/inline-snapshot/
MIT License
343 stars 10 forks source link

New function with simpler API #62

Open alexmojaki opened 2 months ago

alexmojaki commented 2 months ago

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:

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 https://github.com/15r10nk/inline-snapshot/issues/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 commented 2 months ago

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 commented 2 months ago

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 https://github.com/15r10nk/inline-snapshot/issues/57#issuecomment-2018892194:

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 https://github.com/15r10nk/inline-snapshot/issues/48#issuecomment-2018775039, I actually prefer to just apply the changes and review them after.

15r10nk commented 2 months ago

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 commented 2 months ago

Right.

15r10nk commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 #57.

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

marklidenberg commented 2 months ago

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 commented 2 months ago

@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.