diff --git a/crdt-sync/crdt_sync/tests/test_config.py b/crdt-sync/crdt_sync/tests/test_config.py index 1ee62bb..a0932d7 100644 --- a/crdt-sync/crdt_sync/tests/test_config.py +++ b/crdt-sync/crdt_sync/tests/test_config.py @@ -41,10 +41,12 @@ class _StubRemote: accepts it without a suppression. """ - def list_directory(self, path: str) -> list[str]: + def list_directory(self, _path: str) -> list[str]: + """Report an empty remote directory.""" return [] - def get_file_text(self, path: str) -> str | None: + def get_file_text(self, _path: str) -> str | None: + """Report the file as absent.""" return None def put_file_text(self, path: str, text: str, *, message: str) -> None: @@ -54,6 +56,7 @@ class _StubRemote: """Accept and discard a delete.""" def can_access_remote(self) -> bool: + """Report the remote as reachable.""" return True @@ -64,6 +67,7 @@ def _write(path: Path, data: object) -> Path: def test_loads_a_valid_config(tmp_path: Path) -> None: + """Loads a valid config.""" config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) assert config.api_key == "AIzaSyExample" @@ -83,11 +87,13 @@ def test_ignores_the_scaffold_comment_keys(tmp_path: Path) -> None: def test_reports_a_missing_file(tmp_path: Path) -> None: + """Reports a missing file.""" with pytest.raises(ConfigError, match="does not exist"): FirebaseConfig.load(tmp_path / "absent.json") def test_reports_unreadable_json(tmp_path: Path) -> None: + """Reports unreadable JSON.""" path = tmp_path / "firebase.json" path.write_text("{not json", encoding="utf-8") @@ -96,6 +102,7 @@ def test_reports_unreadable_json(tmp_path: Path) -> None: def test_reports_a_non_object_document(tmp_path: Path) -> None: + """Reports a non object document.""" with pytest.raises(ConfigError, match="must contain a JSON object"): FirebaseConfig.load(_write(tmp_path / "firebase.json", ["a list"])) @@ -110,6 +117,7 @@ def test_reports_an_unreadable_path(tmp_path: Path) -> None: @pytest.mark.parametrize("field", sorted(_VALID)) def test_names_a_missing_field(tmp_path: Path, field: str) -> None: + """Names a missing field.""" incomplete = {k: v for k, v in _VALID.items() if k != field} with pytest.raises(ConfigError, match=field): @@ -117,6 +125,7 @@ def test_names_a_missing_field(tmp_path: Path, field: str) -> None: def test_names_an_empty_field(tmp_path: Path) -> None: + """Names an empty field.""" with pytest.raises(ConfigError, match="uid"): FirebaseConfig.load(_write(tmp_path / "firebase.json", {**_VALID, "uid": " "})) @@ -130,6 +139,7 @@ def test_names_an_unfilled_placeholder(tmp_path: Path) -> None: def test_reads_a_password(tmp_path: Path) -> None: + """Reads a password.""" config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) password = tmp_path / "password" password.write_text("hunter2", encoding="utf-8") @@ -147,6 +157,7 @@ def test_strips_a_trailing_newline_from_the_password(tmp_path: Path) -> None: def test_rejects_an_empty_password(tmp_path: Path) -> None: + """Rejects an empty password.""" config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) password = tmp_path / "password" password.write_text(" ", encoding="utf-8") @@ -156,6 +167,7 @@ def test_rejects_an_empty_password(tmp_path: Path) -> None: def test_rejects_a_placeholder_password(tmp_path: Path) -> None: + """Rejects a placeholder password.""" config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) password = tmp_path / "password" password.write_text("PASTE_SYNC_ACCOUNT_PASSWORD_HERE", encoding="utf-8") @@ -165,6 +177,7 @@ def test_rejects_a_placeholder_password(tmp_path: Path) -> None: def test_reports_an_unreadable_password(tmp_path: Path) -> None: + """Reports an unreadable password.""" config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) (tmp_path / "password").mkdir() @@ -191,13 +204,15 @@ def test_client_signs_in_only_without_a_cached_session( sign_ins: list[tuple[str, str]] = [] class _Auth: - def __init__(self, api_key: str, store: object) -> None: + def __init__(self, api_key: str, _store: object) -> None: self.api_key = api_key def has_session(self) -> bool: + """Pretend a cached session exists.""" return True def sign_in(self, email: str, password: str) -> None: + """Record the sign-in attempt.""" sign_ins.append((email, password)) # pragma: no cover - must not run monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) @@ -211,6 +226,7 @@ def test_client_signs_in_when_there_is_no_session( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: + """Client signs in when there is no session.""" config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) password = tmp_path / "password" password.write_text("hunter2", encoding="utf-8") @@ -218,13 +234,15 @@ def test_client_signs_in_when_there_is_no_session( sign_ins: list[tuple[str, str]] = [] class _Auth: - def __init__(self, api_key: str, store: object) -> None: + def __init__(self, api_key: str, _store: object) -> None: self.api_key = api_key def has_session(self) -> bool: + """Pretend no cached session exists.""" return False def sign_in(self, email: str, password_value: str) -> None: + """Record the credentials the caller signed in with.""" sign_ins.append((email, password_value)) monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) @@ -242,13 +260,15 @@ def test_mirror_client_makes_firebase_primary( github = _StubRemote() class _Auth: - def __init__(self, api_key: str, store: object) -> None: + def __init__(self, api_key: str, _store: object) -> None: self.api_key = api_key def has_session(self) -> bool: + """Pretend a cached session exists.""" return True def sign_in(self, email: str, password: str) -> None: + """Record the sign-in attempt.""" raise AssertionError # pragma: no cover - must not run monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) @@ -271,13 +291,15 @@ def test_client_loads_the_shared_config_when_none_is_given( ) class _Auth: - def __init__(self, api_key: str, store: object) -> None: + def __init__(self, api_key: str, _store: object) -> None: self.api_key = api_key def has_session(self) -> bool: + """Pretend a cached session exists.""" return True def sign_in(self, email: str, password: str) -> None: + """Record the sign-in attempt.""" raise AssertionError # pragma: no cover - must not run monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) diff --git a/crdt-sync/crdt_sync/tests/test_firebase.py b/crdt-sync/crdt_sync/tests/test_firebase.py index b6eb24c..eb25b9b 100644 --- a/crdt-sync/crdt_sync/tests/test_firebase.py +++ b/crdt-sync/crdt_sync/tests/test_firebase.py @@ -69,17 +69,22 @@ def _client(*, signed_in: bool = True) -> FirebaseSyncClient: class TestKeyEscaping: + """Key escaping.""" + def test_escapes_a_json_filename_into_a_legal_key(self) -> None: # `.` is illegal in an RTDB key, and the trailing `.json` in a REST # URL is the format suffix -- so the name cannot be stored verbatim. + """Escapes a JSON filename into a legal key.""" assert encode_key("log.json") == "log~2Ejson" def test_leaves_dot_free_device_ids_untouched(self) -> None: + """Leaves dot free device ids untouched.""" assert encode_key("pc") == "pc" uuid = "32c28cf3-c0eb-423f-8bac-9bda9f158054" assert encode_key(uuid) == uuid def test_escapes_every_forbidden_character_and_the_escape_char(self) -> None: + """Escapes every forbidden character and the escape char.""" assert encode_key("a.b$c#d[e]f~g") == "a~2Eb~24c~23d~5Be~5Df~7Eg" @pytest.mark.parametrize( @@ -89,22 +94,29 @@ class TestKeyEscaping: def test_round_trips(self, name: str) -> None: # A name that already looks like an escape must survive: `~` is # escaped first, so `log~2Ejson` is not mistaken for `log.json`. + """Round trips.""" assert decode_key(encode_key(name)) == name def test_escapes_each_path_segment_but_keeps_separators(self) -> None: + """Escapes each path segment but keeps separators.""" assert encode_path("todo-sync/notes/abc.json") == "todo-sync/notes/abc~2Ejson" def test_drops_empty_segments(self) -> None: + """Drops empty segments.""" assert not encode_path("") assert encode_path("/a//b/") == "a/b" class TestGetFileText: + """Get file text.""" + def test_returns_the_stored_text_blob(self) -> None: + """Returns the stored text blob.""" with patch.object(fb.requests, "get", return_value=_response(200, '{"a":1}')): assert _client().get_file_text("ns/devices/pc/log.json") == '{"a":1}' def test_requests_the_escaped_path_with_the_auth_token(self) -> None: + """Requests the escaped path with the auth token.""" with patch.object( fb.requests, "get", return_value=_response(200, "{}") ) as get_mock: @@ -116,10 +128,12 @@ class TestGetFileText: def test_returns_none_for_a_path_never_written_to(self) -> None: # RTDB answers a missing path with literal null and a 200, so this is # benign "no other device has synced yet", not an error. + """Returns none for a path never written to.""" with patch.object(fb.requests, "get", return_value=_response(200, None)): assert _client().get_file_text("ns/devices/phone/log.json") is None def test_raises_when_the_value_is_not_a_text_blob(self) -> None: + """Raises when the value is not a text blob.""" with ( patch.object(fb.requests, "get", return_value=_response(200, {"a": 1})), pytest.raises(FirebaseSyncError, match="not the expected text blob"), @@ -128,6 +142,7 @@ class TestGetFileText: @pytest.mark.parametrize("status", [401, 403]) def test_raises_database_not_found_when_the_rules_reject(self, status: int) -> None: + """Raises database not found when the rules reject.""" with ( patch.object( fb.requests, "get", return_value=_response(status, {"error": "denied"}) @@ -139,6 +154,7 @@ class TestGetFileText: def test_raises_a_plain_sync_error_on_any_other_non_2xx(self) -> None: # Spark quota exhaustion lands here. It must surface, never be # swallowed into a None that reads as "nothing to sync". + """Raises a plain sync error on any other non 2xx.""" with ( patch.object( fb.requests, "get", return_value=_response(429, {"error": "quota"}) @@ -148,6 +164,7 @@ class TestGetFileText: _client().get_file_text("ns/devices/pc/log.json") def test_turns_a_network_failure_into_a_sync_error(self) -> None: + """Turns a network failure into a sync error.""" with ( patch.object( fb.requests, "get", side_effect=requests.ConnectionError("offline") @@ -158,7 +175,10 @@ class TestGetFileText: class TestListDirectory: + """List directory.""" + def test_returns_decoded_keys_and_asks_for_a_shallow_read(self) -> None: + """Returns decoded keys and asks for a shallow read.""" payload = {"pc": True, "phone": True} with patch.object( fb.requests, "get", return_value=_response(200, payload) @@ -170,16 +190,19 @@ class TestListDirectory: def test_decodes_escaped_filenames_back(self) -> None: # The notes app lists filenames, not device dirs: `.json` returns. + """Decodes escaped filenames back.""" with patch.object( fb.requests, "get", return_value=_response(200, {"abc~2Ejson": True}) ): assert _client().list_directory("todo-sync/notes") == ["abc.json"] def test_returns_empty_for_an_unwritten_prefix(self) -> None: + """Returns empty for an unwritten prefix.""" with patch.object(fb.requests, "get", return_value=_response(200, None)): assert not _client().list_directory("ns/devices") def test_raises_on_a_non_2xx_response(self) -> None: + """Raises on a non 2xx response.""" with ( patch.object( fb.requests, "get", return_value=_response(500, {"error": "boom"}) @@ -190,7 +213,10 @@ class TestListDirectory: class TestPutFileText: + """Put file text.""" + def test_writes_the_text_as_a_json_string_leaf(self) -> None: + """Writes the text as a JSON string leaf.""" with ( patch.object(fb.requests, "get", return_value=_response(200, None)), patch.object( @@ -204,6 +230,7 @@ class TestPutFileText: assert put_mock.call_args.args[0].endswith("/log~2Ejson.json") def test_raises_on_a_non_2xx_response(self) -> None: + """Raises on a non 2xx response.""" with ( patch.object( fb.requests, "put", return_value=_response(400, {"error": "bad"}) @@ -213,6 +240,7 @@ class TestPutFileText: _client().put_file_text("ns/devices/pc/log.json", "{}", message="m") def test_turns_a_network_failure_into_a_sync_error(self) -> None: + """Turns a network failure into a sync error.""" with ( patch.object( fb.requests, "put", side_effect=requests.ConnectionError("offline") @@ -223,24 +251,30 @@ class TestPutFileText: class TestGetStringMap: + """Get string map.""" + def test_returns_the_map_with_keys_decoded(self) -> None: + """Returns the map with keys decoded.""" payload = {"pc": "sha-1", "phone": "sha-2"} with patch.object(fb.requests, "get", return_value=_response(200, payload)): assert _client().get_string_map("ns/revs") == payload def test_degrades_to_empty_when_absent(self) -> None: + """Degrades to empty when absent.""" with patch.object(fb.requests, "get", return_value=_response(200, None)): assert not _client().get_string_map("ns/revs") def test_skips_non_string_entries_rather_than_failing(self) -> None: # The revs node is an optimisation; a corrupt one must degrade into # "fetch everything", never into a failed tick. + """Skips non string entries rather than failing.""" with patch.object( fb.requests, "get", return_value=_response(200, {"pc": "sha-1", "b": 42}) ): assert _client().get_string_map("ns/revs") == {"pc": "sha-1"} def test_raises_on_a_non_2xx_response(self) -> None: + """Raises on a non 2xx response.""" with ( patch.object( fb.requests, "get", return_value=_response(500, {"error": "boom"}) @@ -251,7 +285,10 @@ class TestGetStringMap: class TestDeleteFile: + """Delete file.""" + def test_sends_a_delete_to_the_escaped_path(self) -> None: + """Sends a delete to the escaped path.""" with patch.object( fb.requests, "delete", return_value=_response(200, None) ) as delete_mock: @@ -259,6 +296,7 @@ class TestDeleteFile: assert delete_mock.call_args.args[0].endswith("/log~2Ejson.json") def test_raises_on_a_non_2xx_response(self) -> None: + """Raises on a non 2xx response.""" with ( patch.object( fb.requests, "delete", return_value=_response(500, {"error": "boom"}) @@ -268,6 +306,7 @@ class TestDeleteFile: _client().delete_file("ns/devices/pc/log.json") def test_turns_a_network_failure_into_a_sync_error(self) -> None: + """Turns a network failure into a sync error.""" with ( patch.object( fb.requests, "delete", side_effect=requests.ConnectionError("x") @@ -278,17 +317,22 @@ class TestDeleteFile: class TestCanAccessRemote: + """Can access remote.""" + def test_true_when_the_database_root_reads(self) -> None: + """True when the database root reads.""" with patch.object(fb.requests, "get", return_value=_response(200, {"ns": 1})): assert _client().can_access_remote() is True def test_false_when_the_rules_reject_the_token(self) -> None: + """False when the rules reject the token.""" with patch.object( fb.requests, "get", return_value=_response(401, {"error": "denied"}) ): assert _client().can_access_remote() is False def test_false_on_a_network_error(self) -> None: + """False on a network error.""" with patch.object( fb.requests, "get", side_effect=requests.ConnectionError("offline") ): @@ -297,20 +341,26 @@ class TestCanAccessRemote: def test_false_when_not_signed_in_rather_than_raising(self) -> None: # "cannot get a token" is exactly "cannot access the remote", and a # settings Test-connection button must not blow up. + """False when not signed in rather than raising.""" assert _client(signed_in=False).can_access_remote() is False class TestRemoteStoreContract: + """Remote store contract.""" + def test_the_client_satisfies_remote_store(self) -> None: + """The client satisfies remote store.""" assert isinstance(_client(), RemoteStore) def test_database_not_found_is_catchable_as_either_error(self) -> None: + """Database not found is catchable as either error.""" error = DatabaseNotFoundError("gone") assert isinstance(error, FirebaseSyncError) assert isinstance(error, RemoteNotFoundError) assert isinstance(error, RemoteSyncError) def test_a_trailing_slash_on_the_database_url_is_tolerated(self) -> None: + """A trailing slash on the database URL is tolerated.""" client = FirebaseSyncClient(f"{_DATABASE_URL}/", _auth()) with patch.object( fb.requests, "get", return_value=_response(200, "ok") diff --git a/crdt-sync/crdt_sync/tests/test_firebase_auth.py b/crdt-sync/crdt_sync/tests/test_firebase_auth.py index f9c1971..d7c7587 100644 --- a/crdt-sync/crdt_sync/tests/test_firebase_auth.py +++ b/crdt-sync/crdt_sync/tests/test_firebase_auth.py @@ -81,27 +81,36 @@ def _patch_post(*responses: MagicMock) -> object: class TestFirebaseCredentials: + """Firebase credentials.""" + def test_round_trips_through_json(self) -> None: + """Round trips through JSON.""" original = _credentials() restored = FirebaseCredentials.from_json(original.to_json()) assert restored == original def test_is_not_expired_inside_its_lifetime(self) -> None: + """Is not expired inside its lifetime.""" assert _credentials().is_expired_at(_NOW) is False def test_is_expired_once_past_expiry(self) -> None: + """Is expired once past expiry.""" stale = _credentials(valid_for=dt.timedelta(minutes=-1)) assert stale.is_expired_at(_NOW) is True def test_is_expired_inside_the_refresh_skew(self) -> None: # A tick starting now would outlive a token with 2 minutes left, so # it must count as already expired. + """Is expired inside the refresh skew.""" soon = _credentials(valid_for=dt.timedelta(minutes=2)) assert soon.is_expired_at(_NOW) is True class TestSignIn: + """Sign in.""" + def test_stores_the_session(self) -> None: + """Stores the session.""" store = MemoryCredentialStore() body = {"idToken": _ID_1, "refreshToken": _REFRESH_1, "expiresIn": "3600"} with _patch_post(_response(200, body)): @@ -113,6 +122,7 @@ class TestSignIn: assert saved.expires_at == _NOW + dt.timedelta(hours=1) def test_reports_googles_reason_for_a_rejected_password(self) -> None: + """Reports googles reason for a rejected password.""" body = {"error": {"message": "INVALID_LOGIN_CREDENTIALS"}} with ( _patch_post(_response(400, body)), @@ -121,6 +131,7 @@ class TestSignIn: _provider().sign_in("me@example.com", "wrong") def test_reports_a_string_shaped_error_body(self) -> None: + """Reports a string shaped error body.""" with ( _patch_post(_response(400, {"error": "BAD_REQUEST"})), pytest.raises(FirebaseAuthError, match="BAD_REQUEST"), @@ -128,6 +139,7 @@ class TestSignIn: _provider().sign_in("a@b.c", "x") def test_survives_an_error_body_with_no_error_key(self) -> None: + """Survives an error body with no error key.""" with ( _patch_post(_response(400, {"unexpected": "shape"})), pytest.raises(FirebaseAuthError, match="HTTP 400"), @@ -135,6 +147,7 @@ class TestSignIn: _provider().sign_in("a@b.c", "x") def test_survives_a_non_dict_error_body(self) -> None: + """Survives a non dict error body.""" with ( _patch_post(_response(400, ["not", "a", "dict"])), pytest.raises(FirebaseAuthError, match="HTTP 400"), @@ -142,6 +155,7 @@ class TestSignIn: _provider().sign_in("a@b.c", "x") def test_survives_a_non_json_error_body(self) -> None: + """Survives a non JSON error body.""" response = _response(502) response.json = MagicMock(side_effect=ValueError("not json")) with ( @@ -151,6 +165,7 @@ class TestSignIn: _provider().sign_in("a@b.c", "x") def test_turns_a_network_failure_into_an_auth_error(self) -> None: + """Turns a network failure into an auth error.""" with ( patch.object( fa.requests, "post", side_effect=requests.ConnectionError("offline") @@ -161,6 +176,8 @@ class TestSignIn: class TestSignInWithGoogle: + """Sign in with google.""" + @staticmethod def _google_body(**overrides: object) -> dict[str, object]: body: dict[str, object] = { @@ -174,6 +191,7 @@ class TestSignInWithGoogle: return body def test_stores_the_session(self) -> None: + """Stores the session.""" store = MemoryCredentialStore() with _patch_post(_response(200, self._google_body())): _provider(store).sign_in_with_google("google-jwt") @@ -184,6 +202,7 @@ class TestSignInWithGoogle: assert saved.expires_at == _NOW + dt.timedelta(hours=1) def test_returns_the_account_email(self) -> None: + """Returns the account email.""" with _patch_post(_response(200, self._google_body())): email = _provider().sign_in_with_google("google-jwt") assert email == "me@example.com" @@ -192,6 +211,7 @@ class TestSignInWithGoogle: # Firebase leaves `email` out when the Google account has no verified # address. That is not a failure -- the uid check is what matters -- # so the caller gets "" rather than a KeyError. + """Returns an empty email when google omits one.""" body = self._google_body() del body["email"] with _patch_post(_response(200, body)): @@ -200,6 +220,7 @@ class TestSignInWithGoogle: def test_sends_the_credential_form_encoded_in_post_body(self) -> None: # identitytoolkit rejects the IdP credential as a JSON field with # INVALID_IDP_RESPONSE, which reads like a bad token; pin the shape. + """Sends the credential form encoded in post body.""" post = MagicMock(return_value=_response(200, self._google_body())) with patch.object(fa.requests, "post", post): _provider().sign_in_with_google("google-jwt") @@ -208,6 +229,7 @@ class TestSignInWithGoogle: assert "signInWithIdp" in post.call_args.args[0] def test_accepts_the_uid_the_data_belongs_to(self) -> None: + """Accepts the uid the data belongs to.""" store = MemoryCredentialStore() with _patch_post(_response(200, self._google_body())): _provider(store).sign_in_with_google("google-jwt", expected_uid=_UID) @@ -218,6 +240,7 @@ class TestSignInWithGoogle: # omits `idToken`, so an unlinked Google account is signed *up* before # the uid is known. What this guarantees is that no such session is # persisted locally, where it would be denied every read and write. + """Rejects and stores nothing for a different uid.""" store = MemoryCredentialStore() body = self._google_body(localId="someone-else") with ( @@ -228,6 +251,7 @@ class TestSignInWithGoogle: assert store.load() is None def test_reports_googles_reason_for_a_rejected_token(self) -> None: + """Reports googles reason for a rejected token.""" body = {"error": {"message": "INVALID_IDP_RESPONSE"}} with ( _patch_post(_response(400, body)), @@ -237,18 +261,23 @@ class TestSignInWithGoogle: class TestIdToken: + """Id token.""" + def test_fails_loudly_when_not_signed_in(self) -> None: # Never returns None or a stale token: a sync that quietly stops # syncing is the failure mode this design exists to prevent. + """Fails loudly when not signed in.""" with pytest.raises(FirebaseAuthError, match="not signed in"): _provider().id_token() def test_returns_the_stored_token_without_a_network_call(self) -> None: + """Returns the stored token without a network call.""" provider = _provider(MemoryCredentialStore(_credentials())) with patch.object(fa.requests, "post", side_effect=AssertionError("no HTTP")): assert provider.id_token() == _ID_1 def test_refreshes_an_expired_token_and_keeps_the_rotated_one(self) -> None: + """Refreshes an expired token and keeps the rotated one.""" store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) body = { "id_token": _ID_2, @@ -262,6 +291,7 @@ class TestIdToken: assert saved.refresh_token == _REFRESH_2 def test_fails_loudly_when_the_refresh_token_was_revoked(self) -> None: + """Fails loudly when the refresh token was revoked.""" store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) with ( _patch_post(_response(400, {"error": {"message": "TOKEN_EXPIRED"}})), @@ -303,6 +333,7 @@ class TestIdToken: assert store.load() is not None def test_a_server_error_keeps_the_session(self) -> None: + """A server error keeps the session.""" store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) provider = _provider(store) with ( @@ -314,7 +345,10 @@ class TestIdToken: class TestSessionLifecycle: + """Session lifecycle.""" + def test_has_session_is_false_before_sign_in_and_true_after(self) -> None: + """Has session is false before sign in and true after.""" store = MemoryCredentialStore() provider = _provider(store) assert provider.has_session() is False @@ -324,6 +358,7 @@ class TestSessionLifecycle: assert provider.has_session() is True def test_sign_out_clears_the_store(self) -> None: + """Sign out clears the store.""" store = MemoryCredentialStore(_credentials()) provider = _provider(store) assert provider.id_token() == _ID_1 @@ -333,12 +368,16 @@ class TestSessionLifecycle: provider.id_token() def test_defaults_to_the_real_clock_when_none_is_injected(self) -> None: + """Defaults to the real clock when none is injected.""" provider = FirebaseTokenProvider("k", MemoryCredentialStore()) assert provider.has_session() is False class TestFileCredentialStore: + """File credential store.""" + def test_round_trips_through_a_file(self, tmp_path: Path) -> None: + """Round trips through a file.""" store = FileCredentialStore(tmp_path / "nested" / "creds.json") store.save(_credentials()) assert store.load() == _credentials() @@ -346,26 +385,31 @@ class TestFileCredentialStore: def test_writes_the_file_readable_only_by_this_user(self, tmp_path: Path) -> None: # The refresh token is the real secret; it must never be briefly # world-readable between write and chmod. + """Writes the file readable only by this user.""" path = tmp_path / "creds.json" FileCredentialStore(path).save(_credentials()) assert path.stat().st_mode & 0o777 == 0o600 def test_load_returns_none_when_absent(self, tmp_path: Path) -> None: + """Load returns none when absent.""" assert FileCredentialStore(tmp_path / "missing.json").load() is None def test_load_returns_none_for_a_truncated_file(self, tmp_path: Path) -> None: # An interrupted write reads as "not signed in"; the caller's next # step is to sign in again, which repairs it. + """Load returns none for a truncated file.""" path = tmp_path / "creds.json" path.write_text('{"id_token": "a"', encoding="utf-8") assert FileCredentialStore(path).load() is None def test_load_returns_none_when_fields_are_missing(self, tmp_path: Path) -> None: + """Load returns none when fields are missing.""" path = tmp_path / "creds.json" path.write_text('{"id_token": "a"}', encoding="utf-8") assert FileCredentialStore(path).load() is None def test_load_returns_none_for_an_unparsable_expiry(self, tmp_path: Path) -> None: + """Load returns none for an unparsable expiry.""" path = tmp_path / "creds.json" path.write_text( json.dumps( @@ -376,6 +420,7 @@ class TestFileCredentialStore: assert FileCredentialStore(path).load() is None def test_clear_removes_the_file_and_is_idempotent(self, tmp_path: Path) -> None: + """Clear removes the file and is idempotent.""" path = tmp_path / "creds.json" store = FileCredentialStore(path) store.save(_credentials()) @@ -384,6 +429,7 @@ class TestFileCredentialStore: store.clear() def test_overwrites_an_existing_file(self, tmp_path: Path) -> None: + """Overwrites an existing file.""" path = tmp_path / "creds.json" store = FileCredentialStore(path) store.save(_credentials()) diff --git a/crdt-sync/crdt_sync/tests/test_github.py b/crdt-sync/crdt_sync/tests/test_github.py index e60270c..92e5078 100644 --- a/crdt-sync/crdt_sync/tests/test_github.py +++ b/crdt-sync/crdt_sync/tests/test_github.py @@ -51,16 +51,21 @@ def _patch_get_raises() -> object: class TestGetFileText: + """Get file text.""" + def test_returns_decoded_content_on_success(self) -> None: + """Returns decoded content on success.""" encoded = base64.b64encode(b"hello world").decode("ascii") with _patch_get(_response(200, {"content": encoded})): assert _client().get_file_text("devices/pc/log.json") == "hello world" def test_returns_none_for_an_unused_path_on_a_real_repo(self) -> None: + """Returns none for an unused path on a real repo.""" with _patch_get(_response(404), _response(200)): assert _client().get_file_text("devices/phone/log.json") is None def test_raises_repo_not_found_when_the_repo_itself_is_missing(self) -> None: + """Raises repo not found when the repo itself is missing.""" with ( _patch_get(_response(404), _response(404)), pytest.raises(RepoNotFoundError), @@ -68,16 +73,19 @@ class TestGetFileText: _client().get_file_text("devices/pc/log.json") def test_raises_sync_error_on_a_non_2xx_non_404(self) -> None: + """Raises sync error on a non 2xx non 404.""" with _patch_get(_response(500)), pytest.raises(GitHubSyncError): _client().get_file_text("devices/pc/log.json") def test_raises_sync_error_on_a_network_exception(self) -> None: + """Raises sync error on a network exception.""" with _patch_get_raises(), pytest.raises(GitHubSyncError): _client().get_file_text("devices/pc/log.json") def test_treats_a_network_error_during_the_repo_check_as_repo_missing( self, ) -> None: + """Treats a network error during the repo check as repo missing.""" with ( patch.object( _github.requests, @@ -90,20 +98,26 @@ class TestGetFileText: class TestListDirectory: + """List directory.""" + def test_returns_entry_names(self) -> None: + """Returns entry names.""" payload = [{"name": "pc"}, {"name": "phone"}, {"not_a_name": "x"}] with _patch_get(_response(200, payload)): assert _client().list_directory("devices") == ["pc", "phone"] def test_returns_empty_list_when_response_is_not_a_list(self) -> None: + """Returns empty list when response is not a list.""" with _patch_get(_response(200, {"unexpected": "shape"})): assert _client().list_directory("devices") == [] def test_returns_empty_list_for_an_unused_path_on_a_real_repo(self) -> None: + """Returns empty list for an unused path on a real repo.""" with _patch_get(_response(404), _response(200)): assert _client().list_directory("devices") == [] def test_raises_repo_not_found_when_the_repo_itself_is_missing(self) -> None: + """Raises repo not found when the repo itself is missing.""" with ( _patch_get(_response(404), _response(404)), pytest.raises(RepoNotFoundError), @@ -111,12 +125,16 @@ class TestListDirectory: _client().list_directory("devices") def test_raises_sync_error_on_a_non_2xx_non_404(self) -> None: + """Raises sync error on a non 2xx non 404.""" with _patch_get(_response(500)), pytest.raises(GitHubSyncError): _client().list_directory("devices") class TestPutFileText: + """Put file text.""" + def test_creates_a_new_file_with_no_sha_when_none_existed(self) -> None: + """Creates a new file with no sha when none existed.""" with ( _patch_get(_response(404), _response(200)), patch.object( @@ -127,6 +145,7 @@ class TestPutFileText: assert "sha" not in put_mock.call_args.kwargs["json"] def test_updates_an_existing_file_by_including_its_sha(self) -> None: + """Updates an existing file by including its sha.""" with ( _patch_get(_response(200, {"sha": "abc123"})), patch.object( @@ -137,6 +156,7 @@ class TestPutFileText: assert put_mock.call_args.kwargs["json"]["sha"] == "abc123" def test_treats_a_non_string_sha_field_as_absent(self) -> None: + """Treats a non string sha field as absent.""" with ( _patch_get(_response(200, {"sha": 12345})), patch.object( @@ -147,6 +167,7 @@ class TestPutFileText: assert "sha" not in put_mock.call_args.kwargs["json"] def test_raises_repo_not_found_when_checking_sha_on_a_missing_repo(self) -> None: + """Raises repo not found when checking sha on a missing repo.""" with ( _patch_get(_response(404), _response(404)), pytest.raises(RepoNotFoundError), @@ -154,10 +175,12 @@ class TestPutFileText: _client().put_file_text("devices/pc/log.json", "{}", message="m") def test_raises_sync_error_when_the_sha_check_itself_fails(self) -> None: + """Raises sync error when the sha check itself fails.""" with _patch_get(_response(500)), pytest.raises(GitHubSyncError): _client().put_file_text("devices/pc/log.json", "{}", message="m") def test_raises_sync_error_on_a_put_network_exception(self) -> None: + """Raises sync error on a put network exception.""" with ( _patch_get(_response(404), _response(200)), patch.object( @@ -170,6 +193,7 @@ class TestPutFileText: _client().put_file_text("devices/pc/log.json", "{}", message="m") def test_raises_sync_error_on_a_put_non_2xx_response(self) -> None: + """Raises sync error on a put non 2xx response.""" with ( _patch_get(_response(404), _response(200)), patch.object(_github.requests, "put", return_value=_response(422)), @@ -179,15 +203,20 @@ class TestPutFileText: class TestCanAccessRepo: + """Can access repo.""" + def test_true_when_repo_endpoint_ok(self) -> None: + """True when repo endpoint ok.""" with _patch_get(_response(200)): assert _client().can_access_repo() is True def test_false_when_repo_is_missing(self) -> None: + """False when repo is missing.""" with _patch_get(_response(404)): assert _client().can_access_repo() is False def test_false_on_a_network_error(self) -> None: + """False on a network error.""" with _patch_get_raises(): assert _client().can_access_repo() is False @@ -196,14 +225,17 @@ class TestCanAccessRemote: """The ``RemoteStore`` spelling must behave exactly like the legacy name.""" def test_true_when_repo_endpoint_ok(self) -> None: + """True when repo endpoint ok.""" with _patch_get(_response(200)): assert _client().can_access_remote() is True def test_false_when_repo_is_missing(self) -> None: + """False when repo is missing.""" with _patch_get(_response(404)): assert _client().can_access_remote() is False def test_false_on_a_network_error(self) -> None: + """False on a network error.""" with _patch_get_raises(): assert _client().can_access_remote() is False @@ -212,11 +244,13 @@ class TestRemoteStoreContract: """The seam sync talks through must actually be satisfied by the client.""" def test_github_client_is_a_remote_store(self) -> None: + """Github client is a remote store.""" assert isinstance(_client(), RemoteStore) def test_repo_not_found_is_catchable_as_either_error(self) -> None: # Backend-neutral callers catch RemoteNotFoundError; existing GitHub # callers catch GitHubSyncError. One exception must satisfy both. + """Repo not found is catchable as either error.""" error = RepoNotFoundError("gone") assert isinstance(error, GitHubSyncError) assert isinstance(error, RemoteNotFoundError) @@ -224,7 +258,10 @@ class TestRemoteStoreContract: class TestDeleteFile: + """Delete file.""" + def test_deletes_an_existing_file_resolving_its_own_sha(self) -> None: + """Deletes an existing file resolving its own sha.""" with ( _patch_get(_response(200, {"sha": "abc123"})), patch.object(_github.requests, "delete", return_value=_response(200)), @@ -233,6 +270,7 @@ class TestDeleteFile: def test_is_a_no_op_when_the_file_is_absent(self) -> None: # sha GET 404, repo-exists GET 200 -> None sha -> no DELETE sent. + """Is a no op when the file is absent.""" with ( _patch_get(_response(404), _response(200)), patch.object(_github.requests, "delete") as mock_delete, @@ -241,6 +279,7 @@ class TestDeleteFile: mock_delete.assert_not_called() def test_raises_on_a_delete_non_2xx_response(self) -> None: + """Raises on a delete non 2xx response.""" with ( _patch_get(_response(200, {"sha": "abc123"})), patch.object(_github.requests, "delete", return_value=_response(500)), @@ -249,6 +288,7 @@ class TestDeleteFile: _client().delete_file("devices/pc/log.json") def test_raises_on_a_delete_network_error(self) -> None: + """Raises on a delete network error.""" with ( _patch_get(_response(200, {"sha": "abc123"})), patch.object( diff --git a/crdt-sync/crdt_sync/tests/test_hlc.py b/crdt-sync/crdt_sync/tests/test_hlc.py index 381c7ee..68b9c50 100644 --- a/crdt-sync/crdt_sync/tests/test_hlc.py +++ b/crdt-sync/crdt_sync/tests/test_hlc.py @@ -13,35 +13,45 @@ if TYPE_CHECKING: class TestOrdering: + """Ordering.""" + def test_greater_wall_time_wins(self, make_hlc: Callable[..., Hlc]) -> None: + """Greater wall time wins.""" assert make_hlc(200) > make_hlc(100) def test_equal_wall_time_greater_counter_wins( self, make_hlc: Callable[..., Hlc] ) -> None: + """Equal wall time greater counter wins.""" assert make_hlc(100, counter=2) > make_hlc(100, counter=1) def test_equal_wall_time_and_counter_breaks_tie_on_node_id( self, make_hlc: Callable[..., Hlc] ) -> None: + """Equal wall time and counter breaks tie on node id.""" assert make_hlc(100, counter=1, node_id="b") > make_hlc( 100, counter=1, node_id="a" ) def test_identical_clocks_compare_equal(self, make_hlc: Callable[..., Hlc]) -> None: + """Identical clocks compare equal.""" assert make_hlc(100, counter=1, node_id="a") == make_hlc( 100, counter=1, node_id="a" ) class TestNewTick: + """New tick.""" + def test_first_tick_has_counter_zero(self) -> None: + """First tick has counter zero.""" tick = Hlc.new_tick("node-a", wall_time_ms=1000) assert tick == Hlc(wall_time_ms=1000, counter=0, node_id="node-a") def test_advancing_wall_clock_resets_counter( self, make_hlc: Callable[..., Hlc] ) -> None: + """Advancing wall clock resets counter.""" previous = make_hlc(1000, counter=5) tick = Hlc.new_tick("node-a", previous=previous, wall_time_ms=2000) assert tick == Hlc(wall_time_ms=2000, counter=0, node_id="node-a") @@ -49,6 +59,7 @@ class TestNewTick: def test_stalled_wall_clock_increments_counter( self, make_hlc: Callable[..., Hlc] ) -> None: + """Stalled wall clock increments counter.""" previous = make_hlc(1000, counter=5) tick = Hlc.new_tick("node-a", previous=previous, wall_time_ms=1000) assert tick == Hlc(wall_time_ms=1000, counter=6, node_id="node-a") @@ -56,26 +67,32 @@ class TestNewTick: def test_regressed_wall_clock_still_advances_monotonically( self, make_hlc: Callable[..., Hlc] ) -> None: + """Regressed wall clock still advances monotonically.""" previous = make_hlc(1000, counter=5) tick = Hlc.new_tick("node-a", previous=previous, wall_time_ms=500) assert tick == Hlc(wall_time_ms=1000, counter=6, node_id="node-a") def test_defaults_to_the_real_clock_when_unset(self) -> None: + """Defaults to the real clock when unset.""" tick = Hlc.new_tick("node-a") assert tick.node_id == "node-a" assert tick.wall_time_ms > 0 class TestStringRoundTrip: + """String round trip.""" + def test_round_trips_through_to_str_and_from_str( self, make_hlc: Callable[..., Hlc] ) -> None: + """Round trips through to str and from str.""" original = make_hlc(1_751_000_123_456, counter=7, node_id="phone") assert Hlc.from_str(original.to_str()) == original def test_to_str_is_lexicographically_sortable_by_wall_time( self, make_hlc: Callable[..., Hlc] ) -> None: + """To str is lexicographically sortable by wall time.""" earlier = make_hlc(1000) later = make_hlc(2000) assert earlier.to_str() < later.to_str() @@ -83,18 +100,22 @@ class TestStringRoundTrip: def test_to_str_is_lexicographically_sortable_by_counter( self, make_hlc: Callable[..., Hlc] ) -> None: + """To str is lexicographically sortable by counter.""" lower = make_hlc(1000, counter=1) higher = make_hlc(1000, counter=200) assert lower.to_str() < higher.to_str() def test_from_str_rejects_a_missing_z_separator(self) -> None: + """From str rejects a missing z separator.""" with pytest.raises(ValueError, match="not a valid Hlc string"): Hlc.from_str("not-a-valid-clock-string-at-all") def test_from_str_rejects_a_wrong_length_iso_prefix(self) -> None: + """From str rejects a wrong length iso prefix.""" with pytest.raises(ValueError, match="not a valid Hlc string"): Hlc.from_str("2026-07-05T12:00:00Z-0000-node-a") def test_from_str_rejects_a_missing_node_id_separator(self) -> None: + """From str rejects a missing node id separator.""" with pytest.raises(ValueError, match="not a valid Hlc string"): Hlc.from_str("2026-07-05T12:00:00.000Z-0000") diff --git a/crdt-sync/crdt_sync/tests/test_log.py b/crdt-sync/crdt_sync/tests/test_log.py index dc9e7da..2ef6837 100644 --- a/crdt-sync/crdt_sync/tests/test_log.py +++ b/crdt-sync/crdt_sync/tests/test_log.py @@ -13,9 +13,12 @@ if TYPE_CHECKING: class TestMergeLogs: + """Merge logs.""" + def test_returns_the_union_of_disjoint_ids( self, make_hlc: Callable[..., Hlc] ) -> None: + """Returns the union of disjoint ids.""" local = {"a": Record(id="a", fields={"text": ("a", make_hlc(100))})} remote = {"b": Record(id="b", fields={"text": ("b", make_hlc(100))})} assert merge_logs(local, remote) == {**local, **remote} @@ -23,6 +26,7 @@ class TestMergeLogs: def test_merges_a_shared_id_field_by_field( self, make_hlc: Callable[..., Hlc] ) -> None: + """Merges a shared id field by field.""" local = {"a": Record(id="a", fields={"text": ("old", make_hlc(100))})} remote = {"a": Record(id="a", fields={"text": ("new", make_hlc(200))})} merged = merge_logs(local, remote) @@ -31,6 +35,7 @@ class TestMergeLogs: def test_a_delete_on_one_side_survives_the_merge( self, make_hlc: Callable[..., Hlc] ) -> None: + """A delete on one side survives the merge.""" local = { "a": Record(id="a", fields={}, deleted=True, deleted_hlc=make_hlc(200)) } @@ -38,6 +43,7 @@ class TestMergeLogs: assert merge_logs(local, remote)["a"].deleted is True def test_does_not_mutate_either_input(self, make_hlc: Callable[..., Hlc]) -> None: + """Does not mutate either input.""" local = {"a": Record(id="a", fields={"text": ("a", make_hlc(100))})} remote = {"b": Record(id="b", fields={"text": ("b", make_hlc(100))})} local_before = dict(local) @@ -48,6 +54,8 @@ class TestMergeLogs: class TestConvergenceProperties: + """Convergence properties.""" + def _sample_logs(self, make_hlc: Callable[..., Hlc]) -> tuple[Log, Log]: a = { "shared": Record( @@ -64,16 +72,19 @@ class TestConvergenceProperties: return a, b def test_is_commutative(self, make_hlc: Callable[..., Hlc]) -> None: + """Is commutative.""" a, b = self._sample_logs(make_hlc) assert merge_logs(a, b) == merge_logs(b, a) def test_is_idempotent(self, make_hlc: Callable[..., Hlc]) -> None: + """Is idempotent.""" a, _ = self._sample_logs(make_hlc) assert merge_logs(a, a) == a def test_repeated_merge_of_the_same_remote_is_a_no_op( self, make_hlc: Callable[..., Hlc] ) -> None: + """Repeated merge of the same remote is a no op.""" a, b = self._sample_logs(make_hlc) once = merge_logs(a, b) twice = merge_logs(once, b) diff --git a/crdt-sync/crdt_sync/tests/test_mirror.py b/crdt-sync/crdt_sync/tests/test_mirror.py index aaad790..7b8402e 100644 --- a/crdt-sync/crdt_sync/tests/test_mirror.py +++ b/crdt-sync/crdt_sync/tests/test_mirror.py @@ -144,7 +144,10 @@ def _mirror( class TestWrites: + """Writes.""" + def test_go_to_both_backends(self) -> None: + """Go to both backends.""" client, primary, mirror, _ = _mirror() client.put_file_text("ns/pc/log.json", "{}", message="m") assert primary.writes == ["ns/pc/log.json"] @@ -153,17 +156,20 @@ class TestWrites: def test_a_primary_failure_fails_the_tick(self) -> None: # Fail-closed: the primary is authoritative, so a sync that could not # write it must not be reported as successful. + """A primary failure fails the tick.""" client, _, _, _ = _mirror(primary_failing=True) with pytest.raises(RemoteSyncError): client.put_file_text("ns/pc/log.json", "{}", message="m") def test_a_mirror_failure_is_loud_but_survivable(self) -> None: + """A mirror failure is loud but survivable.""" client, primary, _, failures = _mirror(mirror_failing=True) client.put_file_text("ns/pc/log.json", "{}", message="m") assert primary.writes == ["ns/pc/log.json"] assert failures == ["put_file_text ns/pc/log.json"] def test_deletes_behave_the_same_way(self) -> None: + """Deletes behave the same way.""" client, primary, _, failures = _mirror( primary_files={"ns/pc/log.json": "{}"}, mirror_files={"ns/pc/log.json": "{}"}, @@ -174,13 +180,17 @@ class TestWrites: assert failures == ["delete_file ns/pc/log.json"] def test_a_primary_delete_failure_fails_the_tick(self) -> None: + """A primary delete failure fails the tick.""" client, _, _, _ = _mirror(primary_failing=True) with pytest.raises(RemoteSyncError): client.delete_file("ns/pc/log.json") class TestReads: + """Reads.""" + def test_prefer_the_primary_when_it_has_the_file(self) -> None: + """Prefer the primary when it has the file.""" client, _, _, _ = _mirror( primary_files={"ns/pc/log.json": "from-primary"}, mirror_files={"ns/pc/log.json": "from-mirror"}, @@ -191,14 +201,17 @@ class TestReads: # Why reads are not primary-only: a migrated PC must still see an # un-migrated phone's writes, or convergence silently becomes # one-directional with no error raised. + """Fall back to the mirror for an unmigrated device.""" client, _, _, _ = _mirror(mirror_files={"ns/phone/log.json": "from-mirror"}) assert client.get_file_text("ns/phone/log.json") == "from-mirror" def test_return_none_when_neither_backend_has_the_file(self) -> None: + """Return none when neither backend has the file.""" client, _, _, _ = _mirror() assert client.get_file_text("ns/nobody/log.json") is None def test_a_mirror_read_failure_degrades_to_the_primary(self) -> None: + """A mirror read failure degrades to the primary.""" client, _, _, failures = _mirror(mirror_failing=True) assert client.get_file_text("ns/pc/log.json") is None assert failures == ["get_file_text ns/pc/log.json"] @@ -210,13 +223,17 @@ class TestReads: # mirror does not hold the file, so neither side has an answer. A # primary failure alone no longer fails the read -- see # TestPrimaryReadFallback. + """A primary read failure with nothing in the mirror raises.""" client, _, _, _ = _mirror(primary_failing=True) with pytest.raises(RemoteSyncError): client.get_file_text("ns/pc/log.json") class TestListDirectory: + """List directory.""" + def test_unions_devices_from_both_backends(self) -> None: + """Unions devices from both backends.""" client, _, _, _ = _mirror( primary_files={"ns/pc/log.json": "{}"}, mirror_files={"ns/phone/log.json": "{}"}, @@ -224,6 +241,7 @@ class TestListDirectory: assert sorted(client.list_directory("ns")) == ["pc", "phone"] def test_does_not_duplicate_a_device_present_in_both(self) -> None: + """Does not duplicate a device present in both.""" client, _, _, _ = _mirror( primary_files={"ns/pc/log.json": "{}"}, mirror_files={"ns/pc/log.json": "{}"}, @@ -231,6 +249,7 @@ class TestListDirectory: assert client.list_directory("ns") == ["pc"] def test_a_mirror_failure_degrades_to_the_primary_list(self) -> None: + """A mirror failure degrades to the primary list.""" client, _, _, failures = _mirror( primary_files={"ns/pc/log.json": "{}"}, mirror_failing=True ) @@ -242,6 +261,7 @@ class TestListDirectory: # the mirror's devices. This used to raise, which made a primary # outage look like "no devices exist" -- the union read silently # degrading to nothing, precisely when the fallback was needed. + """A primary failure degrades to the mirror list.""" client, _, _, _ = _mirror( primary_failing=True, mirror_files={"ns/phone/log.json": "{}"} ) @@ -250,6 +270,7 @@ class TestListDirectory: def test_both_backends_failing_raises(self) -> None: # With no answer from either side, fail closed: an empty list is # indistinguishable from "no devices", and callers act on that. + """Both backends failing raises.""" client, _, _, _ = _mirror(primary_failing=True, mirror_failing=True) with pytest.raises(RemoteSyncError): client.list_directory("ns") @@ -259,12 +280,14 @@ class TestPrimaryReadFallback: """A failing primary must never hide data the mirror can still serve.""" def test_get_file_text_falls_back_to_the_mirror(self) -> None: + """Get file text falls back to the mirror.""" client, _, _, _ = _mirror( primary_failing=True, mirror_files={"ns/phone/log.json": "{}"} ) assert client.get_file_text("ns/phone/log.json") == "{}" def test_get_file_text_raises_when_both_fail(self) -> None: + """Get file text raises when both fail.""" client, _, _, _ = _mirror(primary_failing=True, mirror_failing=True) with pytest.raises(RemoteSyncError): client.get_file_text("ns/phone/log.json") @@ -274,17 +297,20 @@ class TestPrimaryReadFallback: ) -> None: # A reachable mirror that simply does not hold the file is not an # answer either, since the primary's copy was never read. + """Get file text raises when primary fails and mirror lacks it.""" client, _, _, _ = _mirror(primary_failing=True) with pytest.raises(RemoteSyncError): client.get_file_text("ns/phone/log.json") def test_revision_map_falls_back_to_the_mirror(self) -> None: + """Revision map falls back to the mirror.""" client, _, _, _ = _mirror( primary_failing=True, mirror_files={"ns/revs/phone": "r1"} ) assert client.get_string_map("ns/revs") == {"phone": "r1"} def test_revision_map_raises_when_both_fail(self) -> None: + """Revision map raises when both fail.""" client, _, _, _ = _mirror(primary_failing=True, mirror_failing=True) with pytest.raises(RemoteSyncError): client.get_string_map("ns/revs") @@ -296,6 +322,7 @@ class TestPrimaryReadFallback: # but that is not a failure -- it simply has no revisions to add, so a # working primary's map must still come back. Matches the Dart side, # where the capability check runs inside the guarded closure. + """Revision map tolerates a mirror that cannot bulk read.""" client = MirrorSyncClient( FakeStore({"ns/revs/pc": "r1"}), FakeStoreWithoutBulkRead(), @@ -306,6 +333,7 @@ class TestPrimaryReadFallback: def test_writes_stay_fail_closed_on_a_primary_failure(self) -> None: # The read fix must NOT loosen writes: an unaccepted write has not # happened, so it must still fail the tick. + """Writes stay fail closed on a primary failure.""" client, _, _, _ = _mirror(primary_failing=True) with pytest.raises(RemoteSyncError): client.put_file_text("ns/pc/log.json", "{}", message="m") @@ -314,7 +342,10 @@ class TestPrimaryReadFallback: class TestRevisionMaps: + """Revision maps.""" + def test_merge_both_backends_with_the_primary_winning(self) -> None: + """Merge both backends with the primary winning.""" client, _, _, _ = _mirror( primary_files={"ns/revs/pc": "primary-rev"}, mirror_files={"ns/revs/pc": "mirror-rev", "ns/revs/phone": "old"}, @@ -328,6 +359,7 @@ class TestRevisionMaps: } def test_a_mirror_failure_degrades_to_primary_revisions(self) -> None: + """A mirror failure degrades to primary revisions.""" client, _, _, failures = _mirror( primary_files={"ns/revs/pc": "primary-rev"}, mirror_failing=True ) @@ -335,6 +367,7 @@ class TestRevisionMaps: assert failures == ["get_string_map ns/revs"] def test_a_backend_without_bulk_reads_contributes_nothing(self) -> None: + """A backend without bulk reads contributes nothing.""" client = MirrorSyncClient( FakeStore({"ns/revs/pc": "primary-rev"}), FakeStoreWithoutBulkRead({"ns/revs/phone": "ignored"}), @@ -342,6 +375,7 @@ class TestRevisionMaps: assert client.get_string_map("ns/revs") == {"pc": "primary-rev"} def test_is_empty_when_neither_backend_has_bulk_reads(self) -> None: + """Is empty when neither backend has bulk reads.""" client = MirrorSyncClient( FakeStoreWithoutBulkRead(), FakeStoreWithoutBulkRead() ) @@ -349,9 +383,12 @@ class TestRevisionMaps: class TestLifecycle: + """Lifecycle.""" + def test_can_access_remote_reports_only_the_primary(self) -> None: # A Test-connection button must not report success because the # backend being retired happens to answer. + """Can access remote reports only the primary.""" broken, _, _, _ = _mirror(primary_failing=True) assert broken.can_access_remote() is False healthy, _, _, _ = _mirror(mirror_failing=True) @@ -361,19 +398,24 @@ class TestLifecycle: self, caplog: pytest.LogCaptureFixture, ) -> None: + """A mirror failure with no handler is logged not raised.""" client = MirrorSyncClient(FakeStore(), FakeStore(failing=True)) client.put_file_text("ns/pc/log.json", "{}", message="m") assert "Mirror backend failed" in caplog.text def test_satisfies_the_remote_store_contract(self) -> None: + """Satisfies the remote store contract.""" client, _, _, _ = _mirror() assert isinstance(client, RemoteStore) class TestEndToEndThroughSyncLog: + """End to end through sync log.""" + def test_a_half_migrated_pair_still_converges_both_ways(self) -> None: # phone has not migrated and still writes only to GitHub; pc is on # Firebase with the GitHub mirror. Both must see both records. + """A half migrated pair still converges both ways.""" github = FakeStore() firebase = FakeStore() diff --git a/crdt-sync/crdt_sync/tests/test_record.py b/crdt-sync/crdt_sync/tests/test_record.py index 787c950..ad352be 100644 --- a/crdt-sync/crdt_sync/tests/test_record.py +++ b/crdt-sync/crdt_sync/tests/test_record.py @@ -13,7 +13,10 @@ if TYPE_CHECKING: class TestMergeField: + """Merge field.""" + def test_greater_hlc_wins(self, make_hlc: Callable[..., Hlc]) -> None: + """Greater HLC wins.""" older = ("a", make_hlc(100)) newer = ("b", make_hlc(200)) assert merge_field(older, newer) == newer @@ -22,6 +25,7 @@ class TestMergeField: def test_equal_hlc_keeps_the_first_argument( self, make_hlc: Callable[..., Hlc] ) -> None: + """Equal HLC keeps the first argument.""" clock = make_hlc(100) a = ("a", clock) b = ("a", clock) @@ -29,13 +33,17 @@ class TestMergeField: class TestMergeRecord: + """Merge record.""" + def test_raises_when_ids_differ(self) -> None: + """Raises when ids differ.""" a = Record(id="a", fields={}) b = Record(id="b", fields={}) with pytest.raises(ValueError, match="different ids"): merge_record(a, b) def test_merges_disjoint_fields(self, make_hlc: Callable[..., Hlc]) -> None: + """Merges disjoint fields.""" a = Record(id="x", fields={"text": ("hello", make_hlc(100))}) b = Record(id="x", fields={"priority": ("high", make_hlc(50))}) merged = merge_record(a, b) @@ -47,6 +55,7 @@ class TestMergeRecord: def test_shared_field_keeps_the_later_write( self, make_hlc: Callable[..., Hlc] ) -> None: + """Shared field keeps the later write.""" a = Record(id="x", fields={"text": ("old", make_hlc(100))}) b = Record(id="x", fields={"text": ("new", make_hlc(200))}) merged = merge_record(a, b) @@ -55,12 +64,14 @@ class TestMergeRecord: def test_delete_is_sticky_against_an_older_non_deleted_copy( self, make_hlc: Callable[..., Hlc] ) -> None: + """Delete is sticky against an older non deleted copy.""" deleted = Record(id="x", fields={}, deleted=True, deleted_hlc=make_hlc(200)) stale = Record(id="x", fields={"text": ("resurrected", make_hlc(100))}) assert merge_record(deleted, stale).deleted is True assert merge_record(stale, deleted).deleted is True def test_neither_side_deleted_stays_not_deleted(self) -> None: + """Neither side deleted stays not deleted.""" a = Record(id="x", fields={}) b = Record(id="x", fields={}) merged = merge_record(a, b) @@ -70,6 +81,7 @@ class TestMergeRecord: def test_both_sides_deleted_keeps_the_later_delete_clock( self, make_hlc: Callable[..., Hlc] ) -> None: + """Both sides deleted keeps the later delete clock.""" a = Record(id="x", fields={}, deleted=True, deleted_hlc=make_hlc(100)) b = Record(id="x", fields={}, deleted=True, deleted_hlc=make_hlc(200)) merged = merge_record(a, b) @@ -78,12 +90,14 @@ class TestMergeRecord: def test_both_deleted_but_one_side_missing_a_clock_keeps_the_other( self, make_hlc: Callable[..., Hlc] ) -> None: + """Both deleted but one side missing a clock keeps the other.""" a = Record(id="x", fields={}, deleted=True, deleted_hlc=None) b = Record(id="x", fields={}, deleted=True, deleted_hlc=make_hlc(200)) assert merge_record(a, b).deleted_hlc == make_hlc(200) assert merge_record(b, a).deleted_hlc == make_hlc(200) def test_is_commutative(self, make_hlc: Callable[..., Hlc]) -> None: + """Is commutative.""" record_a = Record(id="x", fields={"text": ("a", make_hlc(100))}) record_b = Record( id="x", fields={"text": ("b", make_hlc(200)), "extra": ("e", make_hlc(1))} @@ -91,22 +105,28 @@ class TestMergeRecord: assert merge_record(record_a, record_b) == merge_record(record_b, record_a) def test_is_idempotent(self, make_hlc: Callable[..., Hlc]) -> None: + """Is idempotent.""" record = Record(id="x", fields={"text": ("a", make_hlc(100))}) assert merge_record(record, record) == record class TestRecordDictRoundTrip: + """Record dict round trip.""" + def test_round_trips_a_record_with_fields( self, make_hlc: Callable[..., Hlc] ) -> None: + """Round trips a record with fields.""" record = Record(id="x", fields={"text": ("hello", make_hlc(100, node_id="pc"))}) assert Record.from_dict(record.to_dict()) == record def test_round_trips_a_deleted_record(self, make_hlc: Callable[..., Hlc]) -> None: + """Round trips a deleted record.""" record = Record(id="x", fields={}, deleted=True, deleted_hlc=make_hlc(200)) assert Record.from_dict(record.to_dict()) == record def test_round_trips_a_record_with_no_delete_clock(self) -> None: + """Round trips a record with no delete clock.""" record = Record(id="x", fields={}, deleted=False, deleted_hlc=None) assert Record.from_dict(record.to_dict()) == record @@ -124,6 +144,7 @@ class TestCrossLanguageWireFormat: """ def test_matches_the_fixture_shared_with_the_dart_package(self) -> None: + """Matches the fixture shared with the dart package.""" record = Record( id="abc123", fields={"text": ("hello", Hlc(wall_time_ms=100, counter=0, node_id="pc"))}, diff --git a/crdt-sync/crdt_sync/tests/test_store.py b/crdt-sync/crdt_sync/tests/test_store.py index 5955f34..2a916c4 100644 --- a/crdt-sync/crdt_sync/tests/test_store.py +++ b/crdt-sync/crdt_sync/tests/test_store.py @@ -30,25 +30,33 @@ def _sample_log(make_hlc: Callable[..., Hlc]) -> Log: class TestDumpLoad: + """Dump load.""" + def test_round_trips_a_log(self, make_hlc: Callable[..., Hlc]) -> None: + """Round trips a log.""" log = _sample_log(make_hlc) assert load_log(dump_log(log)) == log def test_load_raises_on_non_json(self) -> None: # json.loads raises JSONDecodeError (a ValueError subclass). + """Load raises on non JSON.""" with pytest.raises(json.JSONDecodeError): load_log("{not json") def test_load_raises_typeerror_on_wrong_shape(self) -> None: # Valid JSON, but a list where an id->record map is expected. + """Load raises typeerror on wrong shape.""" with pytest.raises(TypeError): load_log("[]") class TestReadWrite: + """Read write.""" + def test_write_then_read_round_trips( self, tmp_path: Path, make_hlc: Callable[..., Hlc] ) -> None: + """Write then read round trips.""" log = _sample_log(make_hlc) path = tmp_path / "nested" / "log.json" write_log(path, log) @@ -57,6 +65,7 @@ class TestReadWrite: def test_write_creates_parent_directories( self, tmp_path: Path, make_hlc: Callable[..., Hlc] ) -> None: + """Write creates parent directories.""" path = tmp_path / "deep" / "nested" / "log.json" write_log(path, _sample_log(make_hlc)) assert path.exists() @@ -64,20 +73,24 @@ class TestReadWrite: def test_write_overwrites_atomically( self, tmp_path: Path, make_hlc: Callable[..., Hlc] ) -> None: + """Write overwrites atomically.""" path = tmp_path / "log.json" write_log(path, {"a": Record(id="a", fields={})}) write_log(path, _sample_log(make_hlc)) assert set(read_log(path)) == {"a", "b"} def test_read_of_missing_file_is_empty(self, tmp_path: Path) -> None: + """Read of missing file is empty.""" assert read_log(tmp_path / "nope.json") == {} def test_read_of_corrupt_file_is_empty(self, tmp_path: Path) -> None: + """Read of corrupt file is empty.""" path = tmp_path / "log.json" path.write_text("{not json", encoding="utf-8") assert read_log(path) == {} def test_read_of_wrong_shape_file_is_empty(self, tmp_path: Path) -> None: + """Read of wrong shape file is empty.""" path = tmp_path / "log.json" path.write_text("[]", encoding="utf-8") assert read_log(path) == {} diff --git a/crdt-sync/crdt_sync/tests/test_sync.py b/crdt-sync/crdt_sync/tests/test_sync.py index 7f6a68c..62bc5c3 100644 --- a/crdt-sync/crdt_sync/tests/test_sync.py +++ b/crdt-sync/crdt_sync/tests/test_sync.py @@ -44,9 +44,12 @@ def _mock_client( class TestSyncLog: + """Sync log.""" + def test_pushes_local_log_when_no_other_devices_have_synced( self, make_hlc: Callable[..., Hlc] ) -> None: + """Pushes local log when no other devices have synced.""" local_log = {"a": Record(id="a", fields={"text": ("hello", make_hlc(100))})} client = _mock_client() @@ -64,6 +67,7 @@ class TestSyncLog: assert client.put_file_text.call_args.args[0] == "devices/pc/log.json" def test_skips_its_own_device_id_when_listing(self) -> None: + """Skips its own device id when listing.""" client = _mock_client( devices=("pc", "phone"), files={"devices/phone/log.json": "{}"} ) @@ -120,6 +124,7 @@ class TestSyncLog: client.get_file_text.assert_called_once_with("devices/pc/log.json") def test_skips_a_device_with_no_pushed_file_yet(self) -> None: + """Skips a device with no pushed file yet.""" client = _mock_client(devices=("phone",), files={}) merged = sync_log( @@ -182,6 +187,7 @@ class TestSyncLog: def test_merges_in_a_remote_devices_entries( self, make_hlc: Callable[..., Hlc] ) -> None: + """Merges in a remote devices entries.""" remote_log = { "b": Record(id="b", fields={"text": ("from phone", make_hlc(100))}) } @@ -202,6 +208,7 @@ class TestSyncLog: assert merged == remote_log def test_uses_a_custom_filename_and_commit_message(self) -> None: + """Uses a custom filename and commit message.""" client = _mock_client() sync_log( diff --git a/crdt-sync/crdt_sync/tests/test_sync_revisions.py b/crdt-sync/crdt_sync/tests/test_sync_revisions.py index 67dde67..5d14749 100644 --- a/crdt-sync/crdt_sync/tests/test_sync_revisions.py +++ b/crdt-sync/crdt_sync/tests/test_sync_revisions.py @@ -171,48 +171,67 @@ def _tick( class TestRevisionOf: + """Revision of.""" + def test_is_stable_for_identical_content(self) -> None: + """Is stable for identical content.""" assert revision_of('{"a":1}') == revision_of('{"a":1}') def test_differs_for_different_content(self) -> None: + """Differs for different content.""" assert revision_of('{"a":1}') != revision_of('{"a":2}') class TestDefaultRevsPath: + """Default revs path.""" + def test_is_a_sibling_of_the_device_directory(self) -> None: + """Is a sibling of the device directory.""" assert default_revs_path("diet-guard-sync/devices") == ("diet-guard-sync/revs") assert default_revs_path("todo-sync/notes") == "todo-sync/revs" def test_falls_back_to_a_child_when_there_is_no_parent(self) -> None: + """Falls back to a child when there is no parent.""" assert default_revs_path("ns") == "ns/revs" class TestSyncState: + """Sync state.""" + def test_round_trips_through_json(self) -> None: + """Round trips through JSON.""" state = SyncState(pushed_rev="abc", peer_revs={"phone": "def"}) assert SyncState.from_json(state.to_json()) == state def test_tolerates_a_missing_peer_map(self) -> None: + """Tolerates a missing peer map.""" assert SyncState.from_json({}).peer_revs == {} def test_drops_malformed_peer_entries(self) -> None: + """Drops malformed peer entries.""" state = SyncState.from_json({"peer_revs": {"phone": 42}}) assert state.peer_revs == {} def test_ignores_a_non_dict_peer_map(self) -> None: + """Ignores a non dict peer map.""" assert SyncState.from_json({"peer_revs": "nonsense"}).peer_revs == {} def test_ignores_a_non_string_pushed_rev(self) -> None: + """Ignores a non string pushed rev.""" assert SyncState.from_json({"pushed_rev": 42}).pushed_rev is None class TestNoOpPushSuppression: + """No op push suppression.""" + def test_pushes_and_publishes_a_revision_on_the_first_tick(self) -> None: + """Pushes and publishes a revision on the first tick.""" remote = FakeRemote() _tick(remote, MemorySyncStateStore(), _log("a", "1")) assert remote.writes == ["ns/devices/pc/log.json", "ns/revs/pc"] def test_a_second_unchanged_tick_writes_nothing(self) -> None: + """A second unchanged tick writes nothing.""" remote = FakeRemote() store = MemorySyncStateStore() local = _log("a", "1") @@ -223,6 +242,7 @@ class TestNoOpPushSuppression: assert not remote.writes def test_a_changed_log_pushes_again(self) -> None: + """A changed log pushes again.""" remote = FakeRemote() store = MemorySyncStateStore() _tick(remote, store, _log("a", "1")) @@ -231,6 +251,7 @@ class TestNoOpPushSuppression: assert "ns/devices/pc/log.json" in remote.writes def test_without_a_state_store_every_tick_pushes(self) -> None: + """Without a state store every tick pushes.""" remote = FakeRemote() local = _log("a", "1") _tick(remote, None, local) @@ -240,7 +261,10 @@ class TestNoOpPushSuppression: class TestPeerDownloadSuppression: + """Peer download suppression.""" + def test_downloads_a_peer_whose_revision_it_has_never_seen(self) -> None: + """Downloads a peer whose revision it has never seen.""" peer = _encode(_log("b", "from-phone", "node-b")) remote = FakeRemote( { @@ -253,6 +277,7 @@ class TestPeerDownloadSuppression: assert "b" in merged def test_skips_the_download_when_the_peer_revision_is_unchanged(self) -> None: + """Skips the download when the peer revision is unchanged.""" peer = _encode(_log("b", "from-phone", "node-b")) remote = FakeRemote( { @@ -269,6 +294,7 @@ class TestPeerDownloadSuppression: assert not remote.reads def test_downloads_again_once_the_peer_publishes_a_new_revision(self) -> None: + """Downloads again once the peer publishes a new revision.""" peer_v1 = _encode(_log("b", "v1", "node-b")) remote = FakeRemote( { @@ -291,6 +317,7 @@ class TestPeerDownloadSuppression: def test_re_downloads_a_peer_whose_push_was_corrupt(self) -> None: # A failed decode must not be remembered as seen, or the corruption # would be permanent. + """Re downloads a peer whose push was corrupt.""" remote = FakeRemote( { "ns/devices/phone/log.json": "not json at all", @@ -306,15 +333,18 @@ class TestPeerDownloadSuppression: def test_downloads_when_the_peer_has_published_no_revision(self) -> None: # A device still running pre-migration code publishes a log but no # revision; it must not be silently ignored. + """Downloads when the peer has published no revision.""" peer = _encode(_log("b", "from-phone", "node-b")) remote = FakeRemote({"ns/devices/phone/log.json": peer}) assert "b" in _tick(remote, MemorySyncStateStore()) def test_skips_a_peer_with_nothing_pushed_yet(self) -> None: + """Skips a peer with nothing pushed yet.""" remote = FakeRemote({"ns/devices/phone/other.txt": "x"}) assert not _tick(remote, MemorySyncStateStore()) def test_never_reads_its_own_device_back(self) -> None: + """Never reads its own device back.""" remote = FakeRemote( { "ns/devices/pc/log.json": _encode(_log("a", "1")), @@ -326,9 +356,12 @@ class TestPeerDownloadSuppression: class TestBackendsWithoutBulkMapReads: + """Backends without bulk map reads.""" + def test_still_sync_correctly_just_without_the_saving(self) -> None: # GitHubSyncClient has no get_string_map, so revision lookup degrades # to "fetch everything" -- correctness must not depend on it. + """Still sync correctly just without the saving.""" peer = _encode(_log("b", "from-phone", "node-b")) remote = FakeRemoteWithoutBulkRead({"ns/devices/phone/log.json": peer}) store = MemorySyncStateStore() @@ -340,9 +373,12 @@ class TestBackendsWithoutBulkMapReads: class TestRevisionPublishingOrder: + """Revision publishing order.""" + def test_publishes_the_log_before_its_revision(self) -> None: # Reversed, a peer would cache "seen rev X" against a log it never # received, and skip it forever. + """Publishes the log before its revision.""" remote = FakeRemote() _tick(remote, MemorySyncStateStore(), _log("a", "1")) assert remote.writes.index("ns/devices/pc/log.json") < remote.writes.index( @@ -353,11 +389,13 @@ class TestRevisionPublishingOrder: # Per-device keys rather than one shared map: a whole-map write would # erase every other device's entry, after which those peers would look # permanently unchanged and never be fetched again. + """Each device writes only its own revision key.""" remote = FakeRemote({"ns/revs/phone": "peer-rev"}) _tick(remote, MemorySyncStateStore(), _log("a", "1")) assert remote.files["ns/revs/phone"] == "peer-rev" def test_an_explicit_revs_path_overrides_the_default(self) -> None: + """An explicit revs path overrides the default.""" remote = FakeRemote() sync_log( client=remote, @@ -373,7 +411,10 @@ class TestRevisionPublishingOrder: class TestTwoDevicesConverge: + """Two devices converge.""" + def test_each_ends_up_with_the_others_records(self) -> None: + """Each ends up with the others records.""" remote = FakeRemote() pc_store, phone_store = MemorySyncStateStore(), MemorySyncStateStore() pc = _tick(remote, pc_store, _log("a", "from-pc", "node-pc")) @@ -392,18 +433,23 @@ class TestTwoDevicesConverge: class TestFileSyncStateStore: + """File sync state store.""" + def test_round_trips_through_a_file(self, tmp_path: Path) -> None: + """Round trips through a file.""" store = FileSyncStateStore(tmp_path / "nested" / "state.json") state = SyncState(pushed_rev="abc", peer_revs={"phone": "def"}) store.save(state) assert store.load() == state def test_load_returns_default_state_when_absent(self, tmp_path: Path) -> None: + """Load returns default state when absent.""" assert FileSyncStateStore(tmp_path / "missing.json").load() == SyncState() def test_load_returns_default_state_for_a_truncated_file( self, tmp_path: Path ) -> None: + """Load returns default state for a truncated file.""" path = tmp_path / "state.json" path.write_text('{"pushed_rev":', encoding="utf-8") assert FileSyncStateStore(path).load() == SyncState() @@ -411,6 +457,7 @@ class TestFileSyncStateStore: def test_load_returns_default_state_for_a_non_dict_file( self, tmp_path: Path ) -> None: + """Load returns default state for a non dict file.""" path = tmp_path / "state.json" path.write_text("[1, 2, 3]", encoding="utf-8") assert FileSyncStateStore(path).load() == SyncState() @@ -418,6 +465,7 @@ class TestFileSyncStateStore: def test_survives_across_processes(self, tmp_path: Path) -> None: # The point of persisting at all: wake_alarm PC is a fresh process # every minute, so an in-memory store would save nothing. + """Survives across processes.""" path = tmp_path / "state.json" remote = FakeRemote() local = _log("a", "1") @@ -429,6 +477,7 @@ class TestFileSyncStateStore: @pytest.mark.parametrize("device_id", ["pc", "phone"]) def test_a_device_publishes_its_revision_under_its_own_id(device_id: str) -> None: + """A device publishes its revision under its own id.""" remote = FakeRemote() _tick(remote, MemorySyncStateStore(), _log("a", "1"), device_id=device_id) assert f"ns/revs/{device_id}" in remote.writes