alunduil / alunduil/zfs-replicate

SNAPSHOTS builds Snapshot directly instead of mapping a fixed dictionary

Aperta
#655 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
enhancement good first issue
Lingua principale
Python
Stelle
24
Fork
6
Merge medio
3h 11m
PR unite (30g)
49

Descrizione

## User story

As a **contributor changing the snapshot strategies**, I want **`SNAPSHOTS` to name the type it constructs** so that **the strategy reads as "draw a `Snapshot` from these fields" rather than "draw a dict, then splat it into a constructor"**.

## Why

`zfs_test/replicate_test/snapshot_test/strategies.py` composes `SNAPSHOTS` in two steps (shown as it stands on #654, which renames the character set):

```python
_SNAPSHOTS_DICT: Dict[str, SearchStrategy[Any]] = {
"filesystem": _FILESYSTEMS,
"name": text(_ROUND_TRIP_SAFE),
"timestamp": integers(),
"previous": none(),
}
SNAPSHOTS = fixed_dictionaries(_SNAPSHOTS_DICT).map(lambda kwargs: Snapshot(**kwargs))
```

`hypothesis.strategies.builds` expresses the same draw as one expression:

```python
SNAPSHOTS = builds(
Snapshot,
filesystem=_FILESYSTEMS,
name=text(_ROUND_TRIP_SAFE),
timestamp=integers(),
previous=none(),
)
```

That retires a variable with one use, its `Dict[str, SearchStrategy[Any]]` annotation, the splat lambda, and the `Any`, `Dict`, `SearchStrategy`, and `fixed_dictionaries` imports that exist only to serve them. In Fowler's terms: Substitute Algorithm on `SNAPSHOTS`, subsuming an Inline Variable on `_SNAPSHOTS_DICT`.

What the change does not buy is static checking of the field names. mypy 2.3.1 under `strict = true` accepts a misspelled keyword in both shapes, checked against a scratch copy of each on 2026-08-21, so a typo stays a runtime `TypeError` either way. Readability is the whole payoff, which is worth saying out loud before someone reaches for `builds` expecting more.

## Acceptance criteria

- [ ] `SNAPSHOTS` is a single `builds(Snapshot, ...)` expression.
- [ ] `_SNAPSHOTS_DICT` and the imports that only served it are gone.
- [ ] The suite passes with no expectation changes, `test_snapshots_vary_filesystem` included, since the draws are meant to be equivalent.

## Out of scope

- The `f"a{x}"` fix and the naming that came with it (#405, #654).
- Giving other strategies modules the same treatment, and the per-symbol test classes in #486.

## Notes

- Target release: none — test-only, no behaviour change.
- Source: review of #654.

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.