Adding getActiveClients() #586
Conversation
| await this.persistence.start(); | ||
| await this.localStore.start(); | ||
| await this.remoteStore.start(); | ||
| async startIfNeeded(): Promise<void> { |
There was a problem hiding this comment.
Why does this need to be idempotent now? I'm not exactly following what changed.
There was a problem hiding this comment.
The way this was implemented before called start() for all clients before execution any of the steps in the tests. That meant by the time client(0).expectNumActiveClients(1) was called, we had already initialized both clients, causing this test to fail.
There was a problem hiding this comment.
Oh... I see. We're creating clients at the start, but not starting them until later... Would it be possible / make sense to just not create them until needed?
I'm a bit worried about making SpecTestRunner deviate too much from other platforms. If nothing else, it seems like we should have some "PORTING NOTE" comments if we're going to change how start() works here...
There was a problem hiding this comment.
I added a PORTING NOTE comment and moved this logic to an inner function (ensureRunner).
This will be used by SharedClientState to retrieve the list of active clients during its startup. Using this list, the SharedClientState can then read all relevant rows in LocalStorage.