Skip to content

Commit 08d704d

Browse files
owenpearsonclaude
andcommitted
docs: record the decisions and the specification faults raised upstream
`decisions.md` covers how the specifications are adopted here: what a derived test may and may not change, and where this SDK's shape forced a choice. `spec-inconsistencies.md` is the report raised against ably/specification. It is kept in the tree because the gating markers point at it by name, so a gated test can be traced to the fault that caused it without leaving the repository. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f807364 commit 08d704d

2 files changed

Lines changed: 693 additions & 0 deletions

File tree

‎test/uts/decisions.md‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
# Decisions
2+
3+
The reasoning behind how the Universal Test Specifications are adopted here.
4+
Deviations from a specification's expected *behaviour* belong in
5+
[deviations.md](deviations.md); this file covers choices about the approach.
6+
7+
## Tests are derived against the async API only
8+
9+
`ably/sync` is generated from `ably/` by `ably/scripts/unasync.py`, a token
10+
rewriter driven by hand-maintained literal-string maps. Those maps name mock
11+
targets individually, so every patch target in a derived test would need an
12+
entry, and a missed entry produces a test that runs against the async class
13+
while appearing to cover the sync one.
14+
15+
The specifications describe client-side behaviour — request formation, response
16+
parsing, state transitions — which is the same code in both variants. Running
17+
the suite twice would establish that the rewriter works, which is a different
18+
question and deserves its own much smaller test.
19+
20+
Derived tests therefore live at `test/uts/`, outside both unasync source globs.
21+
22+
## A mock is an httpx transport, supplied as a client option
23+
24+
`mock_http.md` leaves injection open: "The mechanism for injecting the mock is
25+
implementation-specific and not part of the public API."
26+
27+
The seam is `TestOptions(http_transport=...)`, consumed in `Http.__create_client`.
28+
Serving requests at the transport layer keeps URL construction, header merging,
29+
the host fallback loop and response decoding in the path, which is what the
30+
specifications assert on. It also carries the distinction between a failed
31+
connection and a failed request directly, as httpx raises `ConnectError`,
32+
`ConnectTimeout` and `ReadTimeout` separately.
33+
34+
The alternative, replacing the whole client, would stub out the code under test.
35+
36+
## A mock serves one client rather than being installed globally
37+
38+
The specifications write `install_mock(mock_http)` and warn against passing a
39+
mock to a client, because the SDKs they were first written against hold HTTP
40+
behind a platform singleton. This client builds its HTTP client during
41+
construction and holds it, so a mock is passed as a client option instead.
42+
43+
Derived tests construct the mock first, and `uninstall_mock()` has no
44+
counterpart — `test/uts/conftest.py` closes clients after each test, so a test
45+
whose assertions fail still releases its client.
46+
47+
## A native response body is encoded to match the request
48+
49+
`respond_with(200, {...})` leaves the encoding open. Encoding it as JSON
50+
unconditionally, as the reference implementation does, breaks against this SDK,
51+
where `use_binary_protocol` defaults to `True` and `Response.to_native()`
52+
dispatches on the response content type.
53+
54+
A body given as a dict or list is therefore encoded to match the request's
55+
`Accept` header, and carries the matching `Content-Type`. A body given as
56+
`bytes` or `str` is passed through untouched, so a specification can still pin
57+
the encoding itself.
58+
59+
This keeps the binary protocol on its default setting for the specifications
60+
that exercise it, rather than turning it off across the suite.
61+
62+
## The superseded `queue_*` mock API is implemented
63+
64+
`writing-test-specs.md` lists `mock_http.queue_response()` under "Common
65+
Mistakes to Avoid" in favour of the `onRequest` handler. The specifications have
66+
not followed: `fallback.md`, `request.md` and `rest_client.md` use it heavily.
67+
68+
Implementing it costs little and keeps those three translations literal.
69+
Rewriting each call into a handler and a counter would be the kind of invention
70+
that derived tests exist to avoid.
71+
72+
Stubs are matched by URL, then by host, then consumed first-in-first-out, and a
73+
URL is normalised before matching so that a default port may be given or
74+
omitted.
75+
76+
## Test names carry the spec point, and the Test ID sits above them
77+
78+
A specification's Test ID, `rest/unit/RSC16/returns-server-time-0`, becomes
79+
`test_rsc16_time_returns_server_time`, with the full ID in a `# UTS:` comment
80+
immediately above the function. The comment is the identifier that survives
81+
renaming; the function name makes failures readable without it.
82+
83+
## Timing is driven by client options rather than a fake clock
84+
85+
`enable_fake_timers()` and `ADVANCE_TIME(ms)` have no counterpart here, as there
86+
is no clock seam — `ably/http/http.py` calls `time.time()` directly. Where a
87+
specification advances time, the derived test shortens the interval through a
88+
client option instead, which is what the specifications themselves do for
89+
`fallback_retry_timeout`. Adding a clock seam is left until the realtime specs,
90+
which need one for reconnection timing.
91+
92+
## A TokenDetails payload is recognised by its `token`, not only by `issued`
93+
94+
RSA8c admits "a `TokenRequest` or `TokenDetails` object" from an `authUrl` without
95+
saying how to tell them apart. ably-python discriminated on `issued`, as ably-js
96+
still does, so the `{"token": ..., "expires": ...}` that seven specifications
97+
return was read as a `TokenRequest` and rejected as 40170.
98+
99+
`token` is now accepted as a second discriminator. A `TokenRequest` never carries
100+
one — TE2 makes `keyName`, `nonce` and `mac` its required fields — so this only
101+
widens what is accepted, and the existing `issued` branch is untouched.
102+
103+
It is a deliberate divergence from ably-js, whose derived suite avoids the
104+
question by returning a `text/plain` token string in place of the specification's
105+
JSON body.

0 commit comments

Comments
 (0)