Skip to content

Commit 2264e01

Browse files
Drop implementation-pinning tests and collapse redundant matrices
Remove tests that asserted on source shape (inspect.getsource / ast / __doc__) rather than behavior — each either pinned an implementation detail with no observable contract or duplicated an existing behavioral test. Collapse enum-constant and flag matrices that expanded one property into many near-identical cases into single parametrized loops. Structure pins guarding genuine invariants with no behavioral equivalent (cancel-path ordering, KeyboardInterrupt/SystemExit propagation, cross-package provenance) are kept. No behavioral coverage is removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c4f9e45 commit 2264e01

10 files changed

Lines changed: 54 additions & 347 deletions

tests/test_clusterrequest_decode_no_new_bypass.py

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,12 @@
1-
"""``ClusterRequest.decode_body`` must construct via the dataclass __init__
2-
(``_decoded=True`` kwarg), not ``cls.__new__`` — the bypass is fragile under
3-
frozen/slots/new-field changes and breaks the sibling sentinel-pattern parity.
1+
"""``ClusterRequest.decode_body`` constructs via the dataclass __init__
2+
(``_decoded=True`` kwarg); a V0 request round-trips through that path.
43
"""
54

65
from __future__ import annotations
76

8-
import inspect
9-
107
from dqlitewire.messages.requests import ClusterRequest
118

129

13-
def test_cluster_request_decode_does_not_use_cls_new_bypass() -> None:
14-
source = inspect.getsource(ClusterRequest)
15-
assert "cls.__new__" not in source, (
16-
"ClusterRequest.decode_body must construct via cls(format=..., _decoded=True) — "
17-
"the cls.__new__ bypass is fragile under frozen=True / slots=True / new-field "
18-
"additions and inconsistent with the sibling _decoded_schema pattern."
19-
)
20-
21-
2210
def test_cluster_request_decode_v0_constructor_kwarg_path() -> None:
2311
"""A V0 request constructs directly via ``_decoded=True`` — the decoder's path."""
2412
req = ClusterRequest(format=0, _decoded=True)

tests/test_codec_legacy_leader_direction_guard.py

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,10 @@
66

77
from __future__ import annotations
88

9-
import inspect
10-
119
import pytest
1210

1311
from dqlitewire import PROTOCOL_VERSION_LEGACY
1412
from dqlitewire.codec import (
15-
MessageDecoder,
1613
decode_message,
1714
)
1815
from dqlitewire.constants import RequestType, ResponseType
@@ -21,15 +18,6 @@
2118
from dqlitewire.messages.responses import LeaderResponse
2219

2320

24-
def test_legacy_leader_dispatch_gates_on_direction_not_identity() -> None:
25-
src = inspect.getsource(MessageDecoder.decode_bytes)
26-
assert "not self._is_request" in src, (
27-
"Legacy LeaderResponse dispatch must gate on the direction "
28-
"guard ``not self._is_request``, not solely on the identity "
29-
"coincidence ``msg_class is LeaderResponse``."
30-
)
31-
32-
3321
def _build_legacy_leader_response_bytes(address: str) -> bytes:
3422
"""Legacy LEADER response bytes: header + padded NUL-terminated address, with no
3523
node_id prefix (that field was added in V1)."""

tests/test_codec_uses_header_body_size_property.py

Lines changed: 0 additions & 22 deletions
This file was deleted.

tests/test_constants.py

Lines changed: 46 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -11,40 +11,24 @@ class TestResponseTypeValues:
1111
Reference: github.com/canonical/go-dqlite internal/protocol/constants.go
1212
"""
1313

14-
def test_failure_is_0(self) -> None:
15-
assert ResponseType.FAILURE == 0
16-
17-
def test_node_is_1(self) -> None:
18-
assert ResponseType.LEADER == 1
19-
20-
def test_welcome_is_2(self) -> None:
21-
assert ResponseType.WELCOME == 2
22-
23-
def test_nodes_is_3(self) -> None:
24-
"""Go: ResponseNodes = 3 (cluster server listing)."""
25-
assert ResponseType.SERVERS == 3
26-
27-
def test_db_is_4(self) -> None:
28-
assert ResponseType.DB == 4
29-
30-
def test_stmt_is_5(self) -> None:
31-
assert ResponseType.STMT == 5
32-
33-
def test_result_is_6(self) -> None:
34-
assert ResponseType.RESULT == 6
35-
36-
def test_rows_is_7(self) -> None:
37-
assert ResponseType.ROWS == 7
38-
39-
def test_empty_is_8(self) -> None:
40-
assert ResponseType.EMPTY == 8
41-
42-
def test_files_is_9(self) -> None:
43-
assert ResponseType.FILES == 9
44-
45-
def test_metadata_is_10(self) -> None:
46-
"""Go: ResponseMetadata = 10."""
47-
assert ResponseType.METADATA == 10
14+
@pytest.mark.parametrize(
15+
("name", "value"),
16+
[
17+
("FAILURE", 0),
18+
("LEADER", 1),
19+
("WELCOME", 2),
20+
("SERVERS", 3), # Go: ResponseNodes = 3 (cluster server listing)
21+
("DB", 4),
22+
("STMT", 5),
23+
("RESULT", 6),
24+
("ROWS", 7),
25+
("EMPTY", 8),
26+
("FILES", 9),
27+
("METADATA", 10), # Go: ResponseMetadata = 10
28+
],
29+
)
30+
def test_response_type_value(self, name: str, value: int) -> None:
31+
assert getattr(ResponseType, name) == value
4832

4933
def test_no_node_legacy_as_separate_type(self) -> None:
5034
"""NODE_LEGACY should not exist as a separate type code.
@@ -57,64 +41,34 @@ def test_no_node_legacy_as_separate_type(self) -> None:
5741
class TestRequestTypeValues:
5842
"""Verify RequestType enum matches go-dqlite constants.go."""
5943

60-
def test_leader_is_0(self) -> None:
61-
assert RequestType.LEADER == 0
62-
63-
def test_client_is_1(self) -> None:
64-
assert RequestType.CLIENT == 1
65-
66-
def test_open_is_3(self) -> None:
67-
assert RequestType.OPEN == 3
68-
69-
def test_prepare_is_4(self) -> None:
70-
assert RequestType.PREPARE == 4
71-
72-
def test_exec_is_5(self) -> None:
73-
assert RequestType.EXEC == 5
74-
75-
def test_query_is_6(self) -> None:
76-
assert RequestType.QUERY == 6
77-
78-
def test_finalize_is_7(self) -> None:
79-
assert RequestType.FINALIZE == 7
80-
81-
def test_exec_sql_is_8(self) -> None:
82-
assert RequestType.EXEC_SQL == 8
83-
84-
def test_query_sql_is_9(self) -> None:
85-
assert RequestType.QUERY_SQL == 9
86-
87-
def test_interrupt_is_10(self) -> None:
88-
assert RequestType.INTERRUPT == 10
89-
90-
def test_add_is_12(self) -> None:
91-
assert RequestType.ADD == 12
92-
93-
def test_assign_is_13(self) -> None:
94-
assert RequestType.ASSIGN == 13
95-
96-
def test_remove_is_14(self) -> None:
97-
assert RequestType.REMOVE == 14
98-
99-
def test_dump_is_15(self) -> None:
100-
assert RequestType.DUMP == 15
101-
102-
def test_cluster_is_16(self) -> None:
103-
assert RequestType.CLUSTER == 16
104-
105-
def test_transfer_is_17(self) -> None:
106-
assert RequestType.TRANSFER == 17
107-
108-
def test_describe_is_18(self) -> None:
109-
assert RequestType.DESCRIBE == 18
110-
111-
def test_weight_is_19(self) -> None:
112-
assert RequestType.WEIGHT == 19
113-
114-
def test_connect_is_11(self) -> None:
115-
"""C defines DQLITE_REQUEST_CONNECT = 11 for Raft transport; the Go client omits
116-
it (client-only), but a complete implementation includes it."""
117-
assert RequestType.CONNECT == 11
44+
@pytest.mark.parametrize(
45+
("name", "value"),
46+
[
47+
("LEADER", 0),
48+
("CLIENT", 1),
49+
("OPEN", 3),
50+
("PREPARE", 4),
51+
("EXEC", 5),
52+
("QUERY", 6),
53+
("FINALIZE", 7),
54+
("EXEC_SQL", 8),
55+
("QUERY_SQL", 9),
56+
("INTERRUPT", 10),
57+
# C defines DQLITE_REQUEST_CONNECT = 11 for Raft transport; the Go
58+
# client omits it (client-only), but a complete implementation includes it.
59+
("CONNECT", 11),
60+
("ADD", 12),
61+
("ASSIGN", 13),
62+
("REMOVE", 14),
63+
("DUMP", 15),
64+
("CLUSTER", 16),
65+
("TRANSFER", 17),
66+
("DESCRIBE", 18),
67+
("WEIGHT", 19),
68+
],
69+
)
70+
def test_request_type_value(self, name: str, value: int) -> None:
71+
assert getattr(RequestType, name) == value
11872

11973

12074
class TestPublicExports:

tests/test_decode_message_bypass_via_named_helper.py

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,9 @@
1-
"""``decode_message`` routes its handshake bypass through the named
2-
``MessageDecoder._force_handshake_for_stateless`` rather than writing
3-
``_handshake_done`` / ``_version`` directly, so a future property setter
4-
or observability hook isn't silently bypassed.
1+
"""``decode_message``'s stateless handshake bypass re-validates the version
2+
and leaves a normal request round-trip unchanged.
53
"""
64

75
from __future__ import annotations
86

9-
import inspect
10-
117
import pytest
128

139
from dqlitewire.codec import MessageDecoder, decode_message, encode_message
@@ -16,13 +12,6 @@
1612
from dqlitewire.messages.requests import LeaderRequest
1713

1814

19-
def test_decode_message_does_not_write_handshake_done_directly() -> None:
20-
src = inspect.getsource(decode_message)
21-
assert "decoder._handshake_done = True" not in src
22-
assert "decoder._version = version" not in src
23-
assert "_force_handshake_for_stateless" in src
24-
25-
2615
def test_force_handshake_for_stateless_rejects_unsupported_version() -> None:
2716
"""The bypass helper re-validates the version even if the constructor's
2817
check is later dropped."""

tests/test_decode_row_header_marker_no_per_row_bytes_alloc.py

Lines changed: 2 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,68 +1,10 @@
11
"""``decode_row_header`` and the zero-column fast-path in
2-
``RowsResponse.decode_body`` detect DONE / PART markers via direct
3-
``memoryview[:8] == _ROW_*_MARKER`` equality (buffer-protocol __eq__),
4-
without a per-row ``bytes(data[:8])`` copy.
2+
``RowsResponse.decode_body`` detect DONE / PART markers correctly from both
3+
memoryview- and bytes-backed input, and reject torn markers.
54
"""
65

76
from __future__ import annotations
87

9-
import ast
10-
import inspect
11-
import textwrap
12-
13-
from dqlitewire import tuples as tuples_mod
14-
from dqlitewire.messages import responses as responses_mod
15-
16-
17-
def _decode_row_header_source() -> str:
18-
return textwrap.dedent(inspect.getsource(tuples_mod.decode_row_header))
19-
20-
21-
def _rows_response_decode_body_source() -> str:
22-
return textwrap.dedent(inspect.getsource(responses_mod.RowsResponse.decode_body))
23-
24-
25-
def _has_bytes_call_on_view_prefix(src: str) -> bool:
26-
"""True if the source contains any ``bytes(<expr>[...])`` call."""
27-
tree = ast.parse(src)
28-
for node in ast.walk(tree):
29-
if not isinstance(node, ast.Call):
30-
continue
31-
func = node.func
32-
if isinstance(func, ast.Name) and func.id == "bytes" and node.args:
33-
arg = node.args[0]
34-
if isinstance(arg, ast.Subscript):
35-
return True
36-
if isinstance(arg, ast.Subscript) and isinstance(arg.slice, ast.Slice):
37-
return True
38-
return False
39-
40-
41-
def test_decode_row_header_marker_check_does_not_materialise_per_row_bytes() -> None:
42-
"""The marker-detection arm of ``decode_row_header`` must NOT contain
43-
``bytes(<slice>)``."""
44-
src = _decode_row_header_source()
45-
# Scan only the marker-check arm at the top; tolerate bytes(...) later.
46-
head = "\n".join(src.splitlines()[:25])
47-
assert not _has_bytes_call_on_view_prefix(head), (
48-
"decode_row_header's marker arm allocates bytes(data[:8]) per "
49-
"row; replace with direct ``data[:WORD_SIZE] == _ROW_*_MARKER`` "
50-
"equality (memoryview supports buffer-protocol __eq__ against "
51-
"bytes-like constants without copying)"
52-
)
53-
54-
55-
def test_rows_response_decode_body_zero_column_marker_does_not_materialise_bytes() -> None:
56-
"""The zero-column fast-path in ``RowsResponse.decode_body`` must NOT
57-
call ``bytes(view[...])`` to feed the marker comparison."""
58-
src = _rows_response_decode_body_source()
59-
assert "marker = bytes(" not in src, (
60-
"RowsResponse.decode_body's zero-column marker check still uses "
61-
"bytes(view[offset : offset + WORD_SIZE]); replace with direct "
62-
"``view[offset : offset + WORD_SIZE] == _ROW_*_MARKER`` (memoryview "
63-
"buffer-protocol equality avoids the per-frame copy)"
64-
)
65-
668

679
def test_decode_row_header_marker_detection_works_with_memoryview_input() -> None:
6810
"""DONE and PART markers are identified from memoryview-backed bytes."""

tests/test_decode_value_dispatch_table.py

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,14 @@
1-
"""decode_value dispatches each ValueType via an O(1) lookup keyed by type code, not if/elif."""
1+
"""decode_value dispatches each ValueType to the correct decoder."""
22

33
from __future__ import annotations
44

5-
import ast
6-
import inspect
7-
import textwrap
8-
95
import pytest
106

117
from dqlitewire import types as types_mod
128
from dqlitewire.constants import ValueType
139
from dqlitewire.exceptions import DecodeError
1410

1511

16-
def _decode_value_source() -> str:
17-
return textwrap.dedent(inspect.getsource(types_mod.decode_value))
18-
19-
20-
def test_decode_value_does_not_use_elif_chain_against_value_type() -> None:
21-
src = _decode_value_source()
22-
tree = ast.parse(src)
23-
24-
elif_compare_chain_arms = 0
25-
for node in ast.walk(tree):
26-
if not isinstance(node, ast.Compare):
27-
continue
28-
# Look for ``value_type == ValueType.X`` shape.
29-
if (
30-
isinstance(node.left, ast.Name)
31-
and node.left.id == "value_type"
32-
and len(node.ops) == 1
33-
and isinstance(node.ops[0], ast.Eq)
34-
and len(node.comparators) == 1
35-
and isinstance(node.comparators[0], ast.Attribute)
36-
and isinstance(node.comparators[0].value, ast.Name)
37-
and node.comparators[0].value.id == "ValueType"
38-
):
39-
elif_compare_chain_arms += 1
40-
41-
assert elif_compare_chain_arms <= 1, (
42-
f"decode_value still contains {elif_compare_chain_arms} "
43-
"``value_type == ValueType.X`` comparisons; the dispatch table "
44-
"rewrite should collapse all of them into a single lookup. "
45-
"Up to 1 comparison is tolerated for an explicit fallback arm "
46-
"(e.g. fast-path for the most common type)."
47-
)
48-
49-
5012
@pytest.mark.parametrize(
5113
"value_type, payload, expected_value, expected_consumed",
5214
[

0 commit comments

Comments
 (0)