Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: detect unused snapshots despite xdist #901

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/syrupy/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
)
from typing import (
TYPE_CHECKING,
Any,
Dict,
Iterator,
List,
Expand Down Expand Up @@ -124,6 +125,16 @@
def __contains__(self, key: str) -> bool:
return key in self._snapshot_collections

def serialize(self) -> Dict[str, Any]:
return {k: [c.name for c in v] for k, v in self._snapshot_collections.items()}

Check warning on line 129 in src/syrupy/data.py

View check run for this annotation

Codecov / codecov/patch

src/syrupy/data.py#L129

Added line #L129 was not covered by tests

def merge_serialized(self, data: Dict[str, Any]) -> None:
for location, names in data.items():
snapshot_collection = SnapshotCollection(location=location)
for name in names:
snapshot_collection.add(Snapshot(name))
self.update(snapshot_collection)


@dataclass
class DiffedLine:
Expand Down
31 changes: 31 additions & 0 deletions src/syrupy/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,37 @@
for item in self.ran_items
)

def serialize(self) -> Dict[str, Any]:
return {

Check warning on line 512 in src/syrupy/report.py

View check run for this annotation

Codecov / codecov/patch

src/syrupy/report.py#L512

Added line #L512 was not covered by tests
"discovered": self.discovered.serialize(),
"created": self.created.serialize(),
"failed": self.failed.serialize(),
"matched": self.matched.serialize(),
"updated": self.updated.serialize(),
"used": self.used.serialize(),
"_collected_items": [
{
"nodeid": c.nodeid,
"name": c.name,
"path": str(c.path),
"modulename": c.obj.__module__, # type: ignore[attr-defined]
"methodname": c.obj.__name__, # type: ignore[attr-defined]
}
for c in list(self.collected_items)
],
"_selected_items": {
key: status.value for key, status in self.selected_items.items()
},
}

def merge_serialized(self, data: Dict[str, Any]) -> None:
self.discovered.merge_serialized(data["discovered"])
self.created.merge_serialized(data["created"])
self.failed.merge_serialized(data["failed"])
self.matched.merge_serialized(data["matched"])
self.updated.merge_serialized(data["updated"])
self.used.merge_serialized(data["used"])


@dataclass(frozen=True)
class Expression:
Expand Down
64 changes: 61 additions & 3 deletions src/syrupy/session.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import json
import os
from collections import defaultdict
from dataclasses import (
dataclass,
Expand Down Expand Up @@ -46,6 +48,20 @@
SKIPPED = "skipped"


class _FakePytestObject:
def __init__(self, collected_item: Dict[str, str]) -> None:
self.__module__ = collected_item["modulename"]
self.__name__ = collected_item["methodname"]


class _FakePytestItem:
def __init__(self, collected_item: Dict[str, str]) -> None:
self.nodeid = collected_item["nodeid"]
self.name = collected_item["name"]
self.path = Path(collected_item["path"])
self.obj = _FakePytestObject(collected_item)


@dataclass
class SnapshotSession:
pytest_session: "pytest.Session"
Expand Down Expand Up @@ -127,6 +143,24 @@
except ValueError:
pass # if we don't understand the outcome, leave the item as "not run"

def _merge_collected_items(self, collected_items: List[Dict[str, str]]) -> None:
for collected_item in collected_items:
custom_item = _FakePytestItem(collected_item)
if not any(
t.nodeid == custom_item.nodeid and t.name == custom_item.nodeid
for t in self._collected_items
):
self._collected_items.add(custom_item) # type: ignore[arg-type]

def _merge_selected_items(self, selected_items: Dict[str, str]) -> None:
for key, selected_item in selected_items.items():
if key in self._selected_items:
status = ItemStatus(selected_item)
if status != ItemStatus.NOT_RUN:
self._selected_items[key] = status
else:
self._selected_items[key] = ItemStatus(selected_item)

def finish(self) -> int:
exitstatus = 0
self.flush_snapshot_write_queue()
Expand All @@ -139,16 +173,40 @@
)

if is_xdist_worker():
# TODO: If we're in a pytest-xdist worker, we need to combine the reports
# of all the workers so that the controller can handle unused
# snapshot removal.
worker_count = os.getenv("PYTEST_XDIST_WORKER_COUNT")
with open(".pytest_syrupy_worker_count", "w", encoding="utf-8") as f:
f.write(worker_count) # type: ignore[arg-type]
worker_name = os.getenv("PYTEST_XDIST_WORKER")
with open(

Check warning on line 180 in src/syrupy/session.py

View check run for this annotation

Codecov / codecov/patch

src/syrupy/session.py#L176-L180

Added lines #L176 - L180 were not covered by tests
f".pytest_syrupy_{worker_name}_result",
"w",
encoding="utf-8",
) as f:
json.dump(self.report.serialize(), f, indent=2)

Check warning on line 185 in src/syrupy/session.py

View check run for this annotation

Codecov / codecov/patch

src/syrupy/session.py#L185

Added line #L185 was not covered by tests
return exitstatus
elif is_xdist_controller():
# TODO: If we're in a pytest-xdist controller, merge all the reports.
# Until this is implemented, running syrupy with pytest-xdist is only
# partially functional.
return exitstatus

worker_count = None
try:
with open(".pytest_syrupy_worker_count", encoding="utf-8") as f:
worker_count = f.read()
os.remove(".pytest_syrupy_worker_count")
except FileNotFoundError:
pass

if worker_count:
for i in range(int(worker_count)):
with open(f".pytest_syrupy_gw{i}_result", encoding="utf-8") as f:
data = json.load(f)
self._merge_collected_items(data["_collected_items"])
self._merge_selected_items(data["_selected_items"])
self.report.merge_serialized(data)
os.remove(f".pytest_syrupy_gw{i}_result")

if self.report.num_unused:
if self.update_snapshots:
self.remove_unused_snapshots(
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/test_custom_comparator.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,18 @@ def test_passed_custom(snapshot_custom):


@pytest.mark.xfail(strict=False)
def test_failed_snapshots(generate_snapshots):
def test_failed_snapshots(generate_snapshots, plugin_args):
testdir = generate_snapshots[1]
testdir.makepyfile(test_file=generate_snapshots[2]["failed"])
result = testdir.runpytest("-v")
result = testdir.runpytest("-v", *plugin_args)
result.stdout.re_match_lines((r"1 snapshot failed\."))
assert result.ret == 1


@pytest.mark.xfail(strict=False)
def test_updated_snapshots(generate_snapshots, plugin_args_fails_xdist):
def test_updated_snapshots(generate_snapshots, plugin_args):
_, testdir, initial = generate_snapshots
testdir.makepyfile(test_file=initial["failed"])
result = testdir.runpytest("-v", "--snapshot-update", *plugin_args_fails_xdist)
result = testdir.runpytest("-v", "--snapshot-update", *plugin_args)
result.stdout.re_match_lines((r"1 snapshot updated\."))
assert result.ret == 0
6 changes: 3 additions & 3 deletions tests/integration/test_pytest_extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def test_example(snapshot):
assert result.ret == 0


def test_does_not_print_empty_snapshot_report(testdir, plugin_args_fails_xdist):
def test_does_not_print_empty_snapshot_report(testdir, plugin_args):
testdir.makeconftest("")
testcase_no_snapshots = """
def test_example(snapshot):
Expand All @@ -61,15 +61,15 @@ def test_example(snapshot):
)

result = testdir.runpytest(
"-v", "test_file_no.py", "--snapshot-update", *plugin_args_fails_xdist
"-v", "test_file_no.py", "--snapshot-update", *plugin_args
)
result.stdout.re_match_lines((r".*test_file_no.py.*"))
assert "snapshot report" not in result.stdout.str()
assert "test_file_yes" not in result.stdout.str()
assert result.ret == 0

result = testdir.runpytest(
"-v", "test_file_yes.py", "--snapshot-update", *plugin_args_fails_xdist
"-v", "test_file_yes.py", "--snapshot-update", *plugin_args
)
result.stdout.re_match_lines((r".*test_file_yes.py.*", r".*snapshot report.*"))
assert result.ret == 0
12 changes: 6 additions & 6 deletions tests/integration/test_single_file_multiple_extensions.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from pathlib import Path


def test_multiple_file_extensions(testdir, plugin_args_fails_xdist):
def test_multiple_file_extensions(testdir, plugin_args):
file_extension = "ext2.ext1"

testcase = f"""
Expand All @@ -21,7 +21,7 @@ def test_dot_in_filename(snapshot):

test_file: Path = testdir.makepyfile(test_file=testcase)

result = testdir.runpytest("-v", "--snapshot-update", *plugin_args_fails_xdist)
result = testdir.runpytest("-v", "--snapshot-update", *plugin_args)
result.stdout.re_match_lines((r"1 snapshot generated\."))
assert "snapshots unused" not in result.stdout.str()
assert result.ret == 0
Expand All @@ -34,13 +34,13 @@ def test_dot_in_filename(snapshot):
)
assert snapshot_file.exists()

result = testdir.runpytest("-v", *plugin_args_fails_xdist)
result = testdir.runpytest("-v", *plugin_args)
result.stdout.re_match_lines((r"1 snapshot passed\."))
assert "snapshots unused" not in result.stdout.str()
assert result.ret == 0


def test_class_style(testdir, plugin_args_fails_xdist):
def test_class_style(testdir, plugin_args):
"""
Regression test for https://github.com/syrupy-project/syrupy/issues/717
"""
Expand All @@ -60,7 +60,7 @@ def test_foo(self, snapshot):

test_file: Path = testdir.makepyfile(test_file=testcase)

result = testdir.runpytest("-v", "--snapshot-update", *plugin_args_fails_xdist)
result = testdir.runpytest("-v", "--snapshot-update", *plugin_args)
result.stdout.re_match_lines((r"1 snapshot generated\."))
assert "deleted" not in result.stdout.str()
assert result.ret == 0
Expand All @@ -70,7 +70,7 @@ def test_foo(self, snapshot):
)
assert snapshot_file.exists()

result = testdir.runpytest("-v", *plugin_args_fails_xdist)
result = testdir.runpytest("-v", *plugin_args)
result.stdout.re_match_lines((r"1 snapshot passed\."))
assert "snapshots unused" not in result.stdout.str()
assert result.ret == 0
16 changes: 8 additions & 8 deletions tests/integration/test_snapshot_option_include_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ def test_unused_snapshots_details(
expected_status_code,
run_testfiles_with_update,
testcases,
plugin_args_fails_xdist,
plugin_args,
):
testdir = run_testfiles_with_update(test_file=testcases)
testdir.makepyfile(test_file=testcases["used"])

result = testdir.runpytest(*options, *plugin_args_fails_xdist)
result = testdir.runpytest(*options, *plugin_args)
result.stdout.re_match_lines(
(
r"1 snapshot passed\. 1 snapshot unused\.",
Expand All @@ -85,7 +85,7 @@ def test_unused_snapshots_details(


def test_unused_snapshots_details_multiple_tests(
run_testfiles_with_update, testcases, extra_testcases, plugin_args_fails_xdist
run_testfiles_with_update, testcases, extra_testcases, plugin_args
):
testdir = run_testfiles_with_update(
test_file=testcases, test_second_file=extra_testcases
Expand All @@ -95,7 +95,7 @@ def test_unused_snapshots_details_multiple_tests(
test_second_file="",
)

result = testdir.runpytest("-v", "--snapshot-details", *plugin_args_fails_xdist)
result = testdir.runpytest("-v", "--snapshot-details", *plugin_args)
result.stdout.re_match_lines(
(
r"2 snapshots passed\. 2 snapshots unused\.",
Expand All @@ -108,7 +108,7 @@ def test_unused_snapshots_details_multiple_tests(


def test_unused_snapshots_details_multiple_locations(
run_testfiles_with_update, testcases, extra_testcases, plugin_args_fails_xdist
run_testfiles_with_update, testcases, extra_testcases, plugin_args
):
testdir = run_testfiles_with_update(
test_file=testcases, test_second_file=extra_testcases
Expand All @@ -118,7 +118,7 @@ def test_unused_snapshots_details_multiple_locations(
test_second_file=extra_testcases["extra_a"],
)

result = testdir.runpytest("-v", "--snapshot-details", *plugin_args_fails_xdist)
result = testdir.runpytest("-v", "--snapshot-details", *plugin_args)
result.stdout.re_match_lines_random(
(
r"2 snapshots passed\. 2 snapshots unused\.",
Expand All @@ -131,13 +131,13 @@ def test_unused_snapshots_details_multiple_locations(


def test_unused_snapshots_details_no_details_on_deletion(
run_testfiles_with_update, testcases, plugin_args_fails_xdist
run_testfiles_with_update, testcases, plugin_args
):
testdir = run_testfiles_with_update(test_file=testcases)
testdir.makepyfile(test_file=testcases["used"])

result = testdir.runpytest(
"-v", "--snapshot-details", "--snapshot-update", *plugin_args_fails_xdist
"-v", "--snapshot-details", "--snapshot-update", *plugin_args
)
result.stdout.re_match_lines(
(
Expand Down
12 changes: 6 additions & 6 deletions tests/integration/test_snapshot_option_name.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,25 +32,25 @@ def run_testcases(testdir, testcases):
return testdir, testcases


def test_run_all(run_testcases, plugin_args_fails_xdist):
def test_run_all(run_testcases, plugin_args):
testdir, testcases = run_testcases
result = testdir.runpytest("-v", *plugin_args_fails_xdist)
result = testdir.runpytest("-v", *plugin_args)
result.stdout.re_match_lines("2 snapshots passed")
assert result.ret == 0


def test_failure(run_testcases, plugin_args_fails_xdist):
def test_failure(run_testcases, plugin_args):
testdir, testcases = run_testcases
testdir.makepyfile(test_1=testcases["modified"])
result = testdir.runpytest("-vv", *plugin_args_fails_xdist)
result = testdir.runpytest("-vv", *plugin_args)
result.stdout.re_match_lines("1 snapshot failed. 1 snapshot passed.")
assert result.ret == 1


def test_update(run_testcases, plugin_args_fails_xdist):
def test_update(run_testcases, plugin_args):
testdir, testcases = run_testcases
testdir.makepyfile(test_1=testcases["modified"])
result = testdir.runpytest("-v", "--snapshot-update", *plugin_args_fails_xdist)
result = testdir.runpytest("-v", "--snapshot-update", *plugin_args)
assert "Can not relate snapshot name" not in str(result.stdout)
result.stdout.re_match_lines("1 snapshot passed. 1 snapshot updated.")
assert result.ret == 0
Loading
Loading