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

Do we need snapshot()? #41

Open
max-sixty opened this issue Nov 22, 2023 · 4 comments
Open

Do we need snapshot()? #41

max-sixty opened this issue Nov 22, 2023 · 4 comments
Labels
maybe far future This might be implemented in the far future

Comments

@max-sixty
Copy link

Hi — I'm the creator of pytest-accept, a similar library, but limited to doctests.

Great work on this! Excited to see a well-made effort.

Something I tried to work into pytest-accept was being able to use it without forcing every collaborator to install it.

Would it be feasible to not require foo = snapshot("bar"), and instead just replace any values that are incorrect, so foo = "" is changed to foo = "bar"?

@15r10nk
Copy link
Owner

15r10nk commented Nov 22, 2023

hi, thank you for the kind words 🙂

Why not

I don't think that inline-snapshot can work without snapshot()
The separate snapshot function provides the hook for the implementation to know what it should change.
The following example would not be possible without snapshot()

magic_value=snapshot()

def test_something1():
    assert something1() == magic_value

def test_something2():
    assert something2() == magic_value

There is also another thing which I probably not described in the docs completely. The snapshot function has basically noop semantics, which means that you can do something like this:

try:
    from inline_snapshot import snapshot
except ImportError:
    def snapshot(value):
        return value

I don't know if this help with the "forcing every collaborator to install it" part.

How maybe

The only option I currently see how this could maybe be done is with a complete ast rewrite at import time.

You could transform every assert like follow:

assert something()==5,"message"
# to
__soft_eq_assert(something(),5,"message",some_line_and_col_information)

pytest assert rewriting would not work, but this might be acceptable.

The assert failure could then be reported after the test is completely run. I do something similar here:

@pytest.fixture(autouse=True)
def snapshot_check():
found = _inline_snapshot.found_snapshots = []
yield
missing_values = sum(snapshot._value._old_value is undefined for snapshot in found)
if missing_values and not _inline_snapshot._update_flags.create:
pytest.fail(
"your snapshot is missing a value run pytest with --inline-snapshot=create to create the value",
pytrace=False,
)

@max-sixty
Copy link
Author

I don't know if this help with the "forcing every collaborator to install it" part.

That is a good approach! It not perfectly elegant (there are still snapshot(...)s), but it means the test will still run.


interesting re the ast rewrite. That would be quite intrusive, but I does seem like it should work...

Another possible approach might be: max-sixty/pytest-accept#57, let me know if you have any thoughts...

@15r10nk
Copy link
Owner

15r10nk commented Nov 23, 2023

let me know if you have any thoughts...

As the author noted, this solution might become a problem if the internal pytest api change.

Rewriting the whole module (which would implicit disable the pytest assert rewrite) might be a better option in this case, because you don't have to touch any pytest internals.

Disabling the pytest assert rewriting might not be a problem if you do it only when you run pytest with the --accept option.

@15r10nk 15r10nk added the maybe far future This might be implemented in the far future label Nov 24, 2023
@15r10nk
Copy link
Owner

15r10nk commented Nov 24, 2023

I have maybe an idea how this can be integrated into inline-snapshot but this would require that we can use inline-snapshot and pytest assert rewriting together (assert rewriting is currently disabled when you change snapshots)

It might be possible then to hook into pytest_assetrepr_compare to get the ast of the comparison you want to fix. However this is currently not possible with executing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maybe far future This might be implemented in the far future
Projects
None yet
Development

No branches or pull requests

2 participants