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

Issue when parsing explicit tuples for trimming #77

Closed
pawamoy opened this issue Apr 29, 2024 · 7 comments
Closed

Issue when parsing explicit tuples for trimming #77

pawamoy opened this issue Apr 29, 2024 · 7 comments

Comments

@pawamoy
Copy link
Sponsor Contributor

pawamoy commented Apr 29, 2024

The snapshots, once created, will contain explicit tuples with a single element. It looks like inline-snapshot has trouble parsing them when trimming, because it removes too much code, leaving the test module in a broken state.

import pytest

from inline_snapshot import snapshot, outsource, external

options = {
    "very_very_very_long_option_name": ["value1", "value2"],
    "another_super_extra_long_option_name": [True, False, None],
}


@pytest.mark.parametrize("option1", options["options1"])
@pytest.mark.parametrize("option2", options["options2"])
def test_combinations(option1, option2):
    final_options = {"option1": option1, "option2": option2}
    result = "whatever, not important here"
    assert outsource(result) == snapshot(tuple(final_options.items()))

Run pytest --inline-snapshot=create. The snapshots will look something like this:

snapshots = snapshot(
    {(
            ("very_very_very_long_option_name", "value1"),
            ("another_super_extra_long_option_name", True),
        ): external("36a1a03a6364*.html"), (
            ("very_very_very_long_option_name", "value1"),
            ("another_super_extra_long_option_name", False),
        ): external("bd14e6a60af2*.html"), (
            ("very_very_very_long_option_name", "value1"),
            ("another_super_extra_long_option_name", None),
        ): external("d552c9951139*.html"), (
            ("very_very_very_long_option_name", "value2"),
            ("another_super_extra_long_option_name", True),
        ): external("36a1a03a6364*.html"), (
            ("very_very_very_long_option_name", "value2"),
            ("another_super_extra_long_option_name", False),
        ): external("bd14e6a60af2*.html"), (
            ("very_very_very_long_option_name", "value2"),
            ("another_super_extra_long_option_name", None),
        ): external("d552c9951139*.html")},
)

Optionally format the test module with ruff, and run pytest --inline-snaphost=trim.

It leaves me with something like this in my own test module:

snapshots = snapshot({(
    ("annotations_path", "brief"),
    ("show_signature", True),
    ("show_signature_annotations", False),
    ("signature_crossrefs", True),
    ("separate_signature", True),
    ("unwrap_annotated", False),
): external("c7fb0a797254*.html"), urce", options["show_source"])
# @pytest.mark.parametrize("preload_modules", options["preload_modules"])
# Heading options.
# @pytest.mark.parametrize("heading_level", options["heading_level"])
# @pytest.mark.parametrize("show_root_heading", options["show_root_heading"])
# @pytest.mark.parametrize("show_root_toc_entry", options["show_root_toc_entry"])
# @pytest.mark.parametrize("show_root_full_path", options["show_root_full_path"])
# @pytest.mark.parametrize("show_, eme's templates.

    Parameters:
        identifier: Parametrized identifier.
        handler: Python handler (fixture).
    """
    final_options = {**locals()}
    final_options.pop("handler")
,  for k, v in final_options.items())
        assert outsource(html, suffix=".html") == snapshots[snapshot_key]

, , , , , 

I'll try to create a proper reproduction a bit later.

@pawamoy
Copy link
Sponsor Contributor Author

pawamoy commented Apr 29, 2024

Hmmm I'm actually seeing other issues:

  • broken syntax even when creating snapshots
...
    ("unwrap_annotated", False),
): external("011c334b854b*.html")}{(  # <-- closing and reopening dict? happens both on python 3.8 and 3.10
    ("annotations_path", "brief"),
...
  • always telling that values are missing and to create snapshots (sorted my tuples but no luck)

@pawamoy
Copy link
Sponsor Contributor Author

pawamoy commented Apr 29, 2024

If you want to try it in my own repo:

git clone https://github.com/mkdocstrings/python
git checkout end-to-end-tests
make setup  # install dependencies in venvs (needs uv, install with pipx)

# then you can activate the venv of your choice to test
. .venvs/3.8/bin/activate

# and run the tests
duty test match=end_to_end snapshot=create

# at this point, possible syntax error, try fixing it
# run tests again
duty test match=end_to_end  # probably telling values are missing

# try trimming
duty test match=end_to_end snapshot=trim  # probably breaking syntax

For reference: mkdocstrings/python#157.

@15r10nk
Copy link
Owner

15r10nk commented Apr 29, 2024

Uhh, I think I know the problem. The code range of a tuple ast node does not include the braces. I think this breaks some internal logic in inline-snapshot.

@15r10nk
Copy link
Owner

15r10nk commented Apr 29, 2024

no, this is not the problem. Can you remove -n auto from your pytest invocation if you use inline-snapshot.

    args = [f"--inline-snapshot={snapshot}"] if snapshot else []

    if not snapshot:
        args+=["-n","auto"]

    ctx.run(
        pytest.run(*args, "tests", config_file="config/pytest.ini", select=match, color="yes"),
        title=pyprefix("Running tests"),
        command=f"pytest -c config/pytest.ini -n auto -k{match!r} --color=yes tests",
    )

this should solve your problem. I don't know if it is possible to use inline-snapshot with xdist but I will notify the user if he tries to.

@pawamoy
Copy link
Sponsor Contributor Author

pawamoy commented Apr 29, 2024

Oh damn, yes, it felt like race conditions but I didn't think about pytest-xdist! Good catch. I'll do what you suggest and disable it when inline snapshot modifies files 👍

@pawamoy
Copy link
Sponsor Contributor Author

pawamoy commented Apr 30, 2024

OK, it's working. So sorry for this bad tendency I have to open invalid issues on your repositories 🙇

@pawamoy pawamoy closed this as not planned Won't fix, can't repro, duplicate, stale Apr 30, 2024
@15r10nk
Copy link
Owner

15r10nk commented Apr 30, 2024

nothing to sorry about. I'm currently implementing a check for xdist which should save everyone after you from this mistake.

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

2 participants