Skip to content

Commit

Permalink
fix: improve pytest-xdist compatibility
Browse files Browse the repository at this point in the history
NOTE: When pytest-xdist is detected, we do not remove unused snapshots, since that requires coordination between the workers and the controller. A disclaimer has been added to the README and TODO comments added to the source code to help solve this problem at some point.
  • Loading branch information
Noah Negin-Ulster committed Dec 30, 2022
1 parent e320d7b commit 8739194
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 6 deletions.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,14 @@ If you have decided not to use Syrupy for your project after giving us a try, we

Benchmarks are automatically published to https://tophat.github.io/syrupy/dev/bench/.

## Known Limitations

- `pytest-xdist` support only partially exists. There is no issue when it comes to reads however when you attempt to run `pytest --snapshot-update`, if running with more than 1 process, the ability to detect unused snapshots is disabled. See [#535](https://github.com/tophat/syrupy/issues/535) for more information.
- _Extremely_ large snapshots may fail due to a known Python core library bug. See [#577](https://github.com/tophat/syrupy/issues/577) and [cpython #65452](https://github.com/python/cpython/issues/65452).

_We welcome contributions to patch these known limitations._


## Contributing

Feel free to open a PR or GitHub issue. Contributions welcome!
Expand Down
16 changes: 10 additions & 6 deletions src/syrupy/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
dataclass,
field,
)
from functools import cached_property
from gettext import (
gettext,
ngettext,
Expand Down Expand Up @@ -53,11 +54,14 @@ class SnapshotReport:
information used for removal of unused or orphaned snapshots and fossils.
"""

# Initial arguments to the report
base_dir: str
collected_items: Set["pytest.Item"]
selected_items: Dict[str, bool]
options: "argparse.Namespace"
assertions: List["SnapshotAssertion"]

# All of these are derived from the initial arguments and via walking the filesystem
discovered: "SnapshotFossils" = field(default_factory=SnapshotFossils)
created: "SnapshotFossils" = field(default_factory=SnapshotFossils)
failed: "SnapshotFossils" = field(default_factory=SnapshotFossils)
Expand All @@ -66,9 +70,6 @@ class SnapshotReport:
used: "SnapshotFossils" = field(default_factory=SnapshotFossils)
_provided_test_paths: Dict[str, List[str]] = field(default_factory=dict)
_keyword_expressions: Set["Expression"] = field(default_factory=set)
_collected_items_by_nodeid: Dict[str, "pytest.Item"] = field(
default_factory=dict, init=False
)

@property
def update_snapshots(self) -> bool:
Expand All @@ -82,12 +83,15 @@ def warn_unused_snapshots(self) -> bool:
def include_snapshot_details(self) -> bool:
return bool(self.options.include_snapshot_details)

def __post_init__(self) -> None:
self.__parse_invocation_args()
self._collected_items_by_nodeid = {
@cached_property
def _collected_items_by_nodeid(self) -> Dict[str, "pytest.Item"]:
return {
getattr(item, "nodeid"): item for item in self.collected_items # noqa: B009
}

def __post_init__(self) -> None:
self.__parse_invocation_args()

# We only need to discover snapshots once per test file, not once per assertion.
locations_discovered: DefaultDict[str, Set[Any]] = defaultdict(set)
for assertion in self.assertions:
Expand Down
16 changes: 16 additions & 0 deletions src/syrupy/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
from .constants import EXIT_STATUS_FAIL_UNUSED
from .data import SnapshotFossils
from .report import SnapshotReport
from .utils import (
is_xdist_controller,
is_xdist_worker,
)

if TYPE_CHECKING:
from .assertion import SnapshotAssertion
Expand Down Expand Up @@ -79,6 +83,18 @@ def finish(self) -> int:
assertions=self._assertions,
options=self.pytest_session.config.option,
)

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

if self.report.num_unused:
if self.update_snapshots:
self.remove_unused_snapshots(
Expand Down
10 changes: 10 additions & 0 deletions src/syrupy/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@
from .exceptions import FailedToLoadModuleMember


def is_xdist_worker() -> bool:
worker_name = os.getenv("PYTEST_XDIST_WORKER")
return bool(worker_name and worker_name != "master")


def is_xdist_controller() -> bool:
worker_count = os.getenv("PYTEST_XDIST_WORKER_COUNT")
return bool(worker_count and int(worker_count) > 0 and not is_xdist_worker())


def in_snapshot_dir(path: Path) -> bool:
return SNAPSHOT_DIRNAME in path.parts

Expand Down

0 comments on commit 8739194

Please sign in to comment.