Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP, ECO-4821] Gather further private API usage info #1784

Closed
wants to merge 9 commits into from
Closed

[WIP, ECO-4821] Gather further private API usage info #1784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
caa3f51
remove var shortcuts to helper properties
lawrence-forooghian Jun 6, 2024
150eee0
Declare variables consistently in shared_helper
lawrence-forooghian Jun 6, 2024
dc2f879
Stop using helper.AblyRealtime as a constructor
lawrence-forooghian Jun 10, 2024
5d19fa4
Convert shared helper to a class
lawrence-forooghian Jun 6, 2024
5724018
WIP turn some helper stuff back into static method
lawrence-forooghian Jun 10, 2024
3a10b09
Update tests so that each has its own helper instance
lawrence-forooghian Jun 10, 2024
877620c
Set shared helper on the Test object
lawrence-forooghian Jun 27, 2024
810f356
Move availableTransports and bestTransport inside helper
lawrence-forooghian Jun 11, 2024
d551ba5
Record usages of private APIs in the test suite
lawrence-forooghian Jun 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 74 additions & 2 deletions docs/internal/private-api-usage.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,25 @@ None

### `test/realtime/delta.test.js`

Marked in code.

- `channel._lastPayload.messageId = null`
- to make decoding fail

Note that this test passes an overridden VCDiff decoder, but some SDKs don't use a VCDiff decoder

### `test/realtime/encoding.test.js`

Marked in code.

- `var BufferUtils = Ably.Realtime.Platform.BufferUtils;`
- `var Defaults = Ably.Rest.Platform.Defaults;`
- just used for accessing library’s baked-in protocol version, to pass to `request()`

### `test/realtime/presence.test.js`

Marked in code.

- `var createPM = Ably.protocolMessageFromDeserialized;`
- `var PresenceMessage = Ably.Realtime.PresenceMessage;`
- replacing `channel.sendPresence` with a version that checks the presence message’s client ID
Expand All @@ -47,6 +55,8 @@ None

### `test/realtime/event_emitter.test.js`

Marked in code.

- `eventEmitter.emit('custom');` — RTE6 says that `emit` is internal

### `test/realtime/api.test.js`
Expand All @@ -55,15 +65,18 @@ None

### `test/realtime/crypto.test.js`

Marked in code.

- `var BufferUtils = Ably.Realtime.Platform.BufferUtils;`
- `var msgpack = typeof window == 'object' ? Ably.msgpack : require('@ably/msgpack-js');`
- `Message.encode(testMessage, channelOpts)`
- `Message.decode(encryptedMessage, channelOpts);`
- `Message.fromValues(`
- `expect(channel.channelOptions.cipher.algorithm).to.equal('aes');` — `channel.channelOptions` is not public API

### `test/realtime/failure.test.js`

Marked in code.

- `webSocketConnectTimeout: 50` client option
- replacing `channel.processMessage` to drop `ATTACHED`
- replacing `realtime.connection.connectionManager.activeProtocol.transport.onProtocolMessage` to drop `ACK`
Expand All @@ -75,6 +88,8 @@ None

### `test/realtime/channel.test.js`

Marked in code.

- `var createPM = Ably.protocolMessageFromDeserialized;`
- `expect(channel.channelOptions).to.deep.equal(channelOptions, 'Check requested channel options');` (`channelOptions` isn’t public API)
- listens for `channel._allChannelChanges.on(['update'],`
Expand All @@ -88,13 +103,18 @@ None

### `test/realtime/auth.test.js`

Marked in code.

- `var http = new Ably.Realtime._Http();` — just uses this as an HTTP client to fetch a JWT
- reads `realtime.auth.method` to check it’s `token`
- spies on `rest.time` to count how many times called
- checks `rest.serverTimeOffset`
- spies on `transport.send` to look for an outgoing `AUTH` and check its properties

### `test/realtime/transports.test.js`

Skipped marking in code.

Transports are a JS-specific concept so might not be worth worrying too much about the contents of this file

- `const Defaults = Ably.Rest.Platform.Defaults;`
Expand All @@ -111,12 +131,18 @@ Transports are a JS-specific concept so might not be worth worrying too much abo

### `test/realtime/utils.test.js`

Marked in code.

Ah, I just realised that some of the properties on `shared_helper` actually refer to properties of the library, e.g. `helper.Utils` is actually `Ably.Realtime.Utils`. So perhaps I missed some usages of internal APIs in earlier files. But can figure that out later.

(I’ve addressed this in the context of marking the code; utils stuff is properly marked.)

- this entire file is a test of the internal `utils.getRetryTime(…)` method

### `test/realtime/resume.test.js`

Marked in code.

- `connectionManager.once('transport.active',` and inside the callback it makes an assertion on `transport.params.mode`
- sets a channel’s state: `suspendedChannel.state = 'suspended';`
- sabotages a resume by setting `connection.connectionManager`’s `conectionKey` and `connectionId` to garbage
Expand All @@ -132,6 +158,8 @@ Ah, I just realised that some of the properties on `shared_helper` actually refe

### `test/realtime/message.test.js`

Marked in code.

- `let config = Ably.Realtime.Platform.Config;`
- `var createPM = Ably.protocolMessageFromDeserialized;`
- modifies `transport.send` to check the `clientId` on the outgoing `MESSAGE`
Expand All @@ -141,6 +169,8 @@ Ah, I just realised that some of the properties on `shared_helper` actually refe

### `test/realtime/connection.test.js`

Marked in code.

- creates a `recover` client option which uses knowledge of ably-js’s serialization of recovery key
- spies on `transport.send` to listen for `MESSAGE`, check its properties, and then continue the test
- calls `connectionManager.disconnectAllTransports();`
Expand All @@ -149,6 +179,8 @@ Ah, I just realised that some of the properties on `shared_helper` actually refe

### `test/realtime/init.test.js`

Marked in code.

- accesses `var transport = realtime.connection.connectionManager.activeProtocol.transport.uri` or `.recvRequest.recvUri` to check the `v=3` query parameter
- `expect(realtime.options).to.deep.equal(realtime.connection.connectionManager.options);`
- checks `realtime.connection.connectionManager.httpHosts[0];` to check it’s using correct default host, also checks length of that array
Expand All @@ -165,6 +197,8 @@ None

### `test/realtime/connectivity.test.js`

Marked in code.

- directly calls `new Ably.Realtime._Http().checkConnectivity()` and checks it succeeds (i.e. directly tests this method)

### `test/realtime/reauth.test.js`
Expand All @@ -173,19 +207,27 @@ None

### `test/realtime/sync.test.js`

Marked in code.

- calling `channel.processMessage` to inject an `ATTACHED` with a presence flag, and then a `SYNC`, later on a `PRESENCE`
- spies on `channel.processMessage` to, after processing a received `SYNC`, inject a `PRESENCE`

### `test/browser/simple.test.js`

Skipped marking in code since it’s browser-specific.

- checks whether a transport is available using `transport in Ably.Realtime.ConnectionManager.supportedTransports(Ably.Realtime._transports)`

### `test/browser/http.test.js`

Skipped marking in code since it’s browser-specific.

- changes `Ably.Rest.Platform.Config.xhrSupported` to false to make it use Fetch

### `test/browser/connection.test.js`

Skipped marking in code since it’s browser-specific.

(I guess that this is a test that we might not include in the unified test suite.)

- uses knowledge of library’s usage of `window.sessionStorage` for transport preference and recovery key
Expand All @@ -197,6 +239,8 @@ None

### `test/browser/modular.test.js`

Skipped marking in code since it’s browser-specific.

Another file that probably wouldn’t be part of a unified test suite.

- `const BufferUtils = BaseRest.Platform.BufferUtils;`
Expand Down Expand Up @@ -227,10 +271,14 @@ N/A

### `test/common/modules/client_module.js`

- uses `Ably.Realtime.Utils` for its `mixin` function
Marked in code.

- uses `Ably.Realtime.Utils` for its `mixin` and `copy` function

### `test/common/modules/testapp_manager.js`

Marked in code.

- uses `ably.Realtime.Platform.BufferUtils`

### `test/common/modules/testapp_module.js`
Expand All @@ -239,6 +287,8 @@ None

### `test/common/modules/shared_helper.js`

Marked in code.

- `var utils = clientModule.Ably.Realtime.Utils;`
- `var platform = clientModule.Ably.Realtime.Platform;`
- uses `platform.Config.nextTick()`
Expand Down Expand Up @@ -314,15 +364,21 @@ N/A

### `test/rest/bufferutils.test.js`

Skipped marking in code.

This file is a unit test of `Ably.Realtime.Platform.BufferUtils`; something we wouldn’t include in a unified test suite.

### `test/rest/presence.test.js`

Marked in code.

- `var BufferUtils = Ably.Realtime.Platform.BufferUtils;`
- I’m going to stop mentioning the use of BufferUtils as a test util now; the pattern is clear and not hard to fix.

### `test/rest/fallbacks.test.js`

Marked in code.

- checks `rest._currentFallback.{host, validUntil}` to check that the working fallback has been stored correctly
- modifies `rest._currentFallback.validUntil` to check library correctly forgets stored fallback

Expand All @@ -344,12 +400,16 @@ None

### `test/rest/auth.test.js`

Marked in code.

- ditto will stop mentioning usage of `Utils` for stuff that will be pretty easy to replace

None

### `test/rest/http.test.js`

Marked in code.

- accesses `Ably.Rest.Platform.Defaults` to check its `version` is being used to populate `Ably-Agent`
- spies on `rest.http.do` to make assertions about request headers
- replaces `rest.http.do` to simulate a 204 response
Expand All @@ -360,25 +420,35 @@ None

### `test/rest/push.test.js`

Marked in code.

None

### `test/rest/message.test.js`

Marked in code.

- spies on `channel._publish` to verify that client does / doesn’t add a `clientId`
- ditto to check that idempotent REST publishing generates message IDs
- ditto to check `params`
- overrides `Ably.Rest._Http.doUri` to fake a publish error

### `test/rest/init.test.js`

Marked in code.

- accesses various properties of `rest.options` to check the effect of passing various things to the constructor

### `test/rest/history.test.js`

Marked in code.

None

### `test/rest/defaults.test.js`

Marked in code.

This appears to be a unit test of the `Defaults` class’s `normaliseOptions()`, `getHosts()`, and `getPort()` methods. But I imagine it’s actually providing the test suite’s coverage of a bunch of spec points which aren’t written in terms of this API.

### `test/rest/status.test.js`
Expand All @@ -387,6 +457,8 @@ None

### `test/rest/request.test.js`

Marked in code.

- overrides `rest.http.do()` to check `X-Ably-Version` request header

### `test/package/browser/template/server/resources/runTest.js`
Expand Down
Loading
Loading