Skip to content

/time is mocked as {"time": N} in 48 places across six rest/unit specs; the endpoint returns [server_time_ms], so a typed SDK cannot derive those tests as written #524

Description

@owenpearson

Summary

The /time endpoint answers with a single-element array holding the server time in milliseconds. uts/rest/unit/time.md and uts/rest/unit/logging.md mock it that way; six other specs mock it as a JSON object {"time": 1234567890000}. The split is 48 object-shaped sites against 9 array-shaped ones, and uts/rest/unit/helpers/mock_http.md — the document the other specs point at for the mock contract — is on the wrong side, which is the likely origin.

A dynamically-typed SDK can absorb this; a typed one cannot. Both SDKs that have derived these specs rewrote the fixture — ably-pubsub-js silently, at every site. This asks for a mechanical replacement throughout, starting with the mock contract file.

What the spec says today

specifications/features.md:91:

  • (RSC16) RestClient#time function sends a get request to the /time path of the REST endpoint as specified in RSC25 and returns the server time in milliseconds since epoch or as a Date/Time object where suitable

and the IDL at specifications/features.md:2134:

time() => io Time // RSC16

Both constrain the value the SDK returns, not the wire body. specifications/protocol.md does not mention /time at all — it is a REST endpoint, outside that document's scope.

So the array shape is not stated normatively anywhere in this repo. It is documented only in the UTS files that get it right, and in the SDKs that parse it — for instance ably-python's ably/rest/rest.py:94-98, which reads r.to_native()[0]. That is itself part of the problem: the mock contract is the de facto specification of this response body, and it currently specifies the wrong one. If the intent is that RSC16 should say what /time returns on the wire, that would be worth adding; the ask below does not depend on it.

The defect

uts/rest/unit/helpers/mock_http.md:63-75, the handler-configuration example, keys explicitly on the path:

onRequest: (req) => {
  IF req.url.path == "/time":
    req.respond_with(200, {"time": 1234567890000})
  ELSE:
    req.respond_with(404, {"error": {"code": 40400}})
}

Against uts/rest/unit/time.md:42, which is correct:

onRequest: (req) => {
  captured_requests.push(req)
  req.respond_with(200, [server_time_ms])
}

Occurrences, recounted against d9a04ca

File Object-shaped {"time": N} Lines
rest/unit/fallback.md 29 134, 163, 192, 233, 265, 296, 332, 365, 418, 455, 457, 503, 505, 635, 661, 690, 719, 750, 779, 900, 975, 1004, 1033, 1073, 1110, 1165, 1239, 1279, 1319
rest/unit/rest_client.md 9 51, 88, 123, 163, 280, 439, 524, 531, 574
rest/unit/request_endpoint.md 4 28, 58, 91, 130
rest/unit/helpers/mock_http.md 4 70, 100, 125, 156
rest/unit/request.md 1 788
rest/unit/auth/authorize.md 1 379
total 48
File Array-shaped [N] Lines
rest/unit/logging.md 5 42, 74, 116, 154, 190
rest/unit/time.md 4 42, 86, 131, 174
rest/unit/fallback.md 3 561, 567, 590
total 12

fallback.md contradicts itself: its newest test (RSC15f/expired-not-resurrected-2, fallback.md:535) uses req.respond_with(200, [1000]) while the 29 older setups in the same file use the object.

Two sites are not plain fixtures and are worth calling out:

  • rest_client.md:280 wraps the object for the msgpack protocol test: body: msgpack_encode({ "time": 1234567890000 }).
  • request.md:788 (RSC19d/non-array-response-handling-7, heading at :779) queues the object because it wants a non-array response — but it fetches it from /time, which is the one path that can never return one. Its assertion at :802 hedges accordingly: ASSERT items.length == 1 OR items["time"] == 1234567890000, followed by # Implementation may vary. This one needs a different path, not a different body; see the fix below.

The same object shape also appears three times in the authoring guide, uts/docs/writing-test-specs.md:138, :215 and :1037 (the last inside an IF req.url.path CONTAINS "/time" branch), which is presumably how it spread.

Consequence

An SDK whose time() indexes the array fails at the first of these tests it derives. ably-python returns r.to_native()[0]; against {"time": 1234567890000} that is KeyError: 0, which @catch_all converts into a bogus AblyException. Nothing about the failure points at the fixture. A JavaScript implementation reading response[0] gets undefined rather than an exception, so the same defect degrades to a wrong value or a silently vacuous assertion depending on what the test goes on to check — which is why this has survived 48 sites without being reported.

Concretely, in this branch ably-python had to carry the correction in the derived test, test/uts/rest/unit/rest_client_test.py:18-21:

# NOTE: the spec stubs /time with {"time": 1234567890000}. The endpoint answers with an array
# holding the time, which is what time.md stubs and what the SDK reads, so the value is kept
# and the shape corrected.
TIME_RESPONSE = [SERVER_TIME_MS]

Cross-SDK

SDK Derived these specs What it wrote Recorded the deviation
ably-python (this branch) yes [1234567890000] yes, test/uts/deviations.md:57-62
ably-pubsub-js yes [1234567890000] no
ably-cocoa only time.md, which is already correct n/a n/a
ably-pubsub-java no REST unit specs n/a n/a

ably-pubsub-js rewrote the fixture at every site it derived and recorded nothing. test/uts/rest/unit/rest_client.test.ts:35-41, its RSC7e test, derived from rest_client.md:51:

const mock = new MockHttpClient({
  onConnectionAttempt: (conn) => conn.respond_with_success(),
  onRequest: (req) => {
    captured.push(req);
    req.respond_with(200, [1234567890000]);
  },
});

The same substitution appears at rest_client.test.ts:39, 65, 97, 183, 205, 303, at request_endpoint.test.ts:33, 58, 87, 121, and throughout fallback.test.ts — and fallback.test.ts:50 asserts expect(result).to.equal(1234567890000), which only holds for the array. Its test/uts/deviations.md has no entry for /time.

Proposed fix

Mechanical, in this order:

  1. uts/rest/unit/helpers/mock_http.md:70, 100, 125, 156 — replace {"time": 1234567890000} with [1234567890000]. This is the contract document; fixing it first makes the rest consistent with what it says.
  2. uts/docs/writing-test-specs.md:138, 215, 1037 — same replacement, so new specs are not written against the wrong shape. (:1037 is the illustrative {"time": ...} placeholder.)
  3. uts/rest/unit/fallback.md, rest_client.md, request_endpoint.md, auth/authorize.md — same replacement at the 43 lines tabulated above, including rest_client.md:280, which becomes msgpack_encode([1234567890000]).
  4. uts/rest/unit/request.md:788-802 — not a body substitution. The test's subject is a non-array response, so point it at a path that returns an object (/channels/<name> channel status, say) and replace the hedged assertion at :802 with a single definite one. Alternatively, if RSC19d is meant to be tested against a bare scalar rather than an object, /time can stay and the assertion becomes ASSERT items.length == 1 AND items[0] == 1234567890000 — but that is a different test from the one the heading describes.

No features.md change is required by any of the above. If you want the response shape pinned normatively rather than only by the mock contract, that is a separate, smaller ask on RSC16.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions