Calls test runner directly from host process #62
Conversation
|
@jugglinmike @gnarf this isn't quite ready to be reviewed yet but I would appreciate if either of you could pull this branch down and test if you can run it on NVDA? I've only tested this with Voiceover so far. |
|
Hey @ChrisC; I'm working on testing this in Windows, but I'm having trouble getting it up and running locally (the harness is reporting an error connecting to at-driver). I'll give it another shot tomorrow. |
|
(I should add that my problems can't be related to anything in this patch because I'm starting with the main branch as a base-line.) |
|
Good news: as of commit b04b4b8, this branch produces equivalent output in Windows. Keep it up! |
src/host/main.js
Outdated
| await agent.start({ referenceBaseUrl: serverDirectory.baseUrl }); | ||
| const timesOption = getTimesOption(options); | ||
|
|
||
| const emitter = new EventEmitter(); |
There was a problem hiding this comment.
An event emitter seems like more than we need since we're only looking to resolve a single promise at the first occurrence of a single event. Think we can get away with leaking abortSignal's resolver function and invoking it directly?
There was a problem hiding this comment.
Let me know if my latest commits address this! I was able to remove the emitter and just trigger via a promise resolver.
src/runner/mock-test-runner.js
Outdated
| * @returns {AriaATCIRunner.MockOptions} | ||
| */ | ||
| export function runnerMockOptions({ mock, mockOpenPage }) { | ||
| if (mock === undefined && mockOpenPage) { |
There was a problem hiding this comment.
In order to support this branch, the type for mock should read something like @param {boolean} [options.mock], and the same goes for the optional mockOpenPage parameter. That said, I'm wondering if we need to implement these branches in the first place. I personally tend to prefer explicitness over brevity, and in this case, the shorthand doesn't seem to be in use--the types used in the call site indicate that these values are always specified.
Co-authored-by: jugglinmike <[email protected]>
to stop drivers.
As part of an effort to remove the IPC abstractions in favor of a single-process architecture in the harness, this draft PR calls the test runner functions directly from the main process without kicking off an "agent" child process.
As far as naming goes, I opted to keep the overall file and folder naming and organization conventions in place, and simply renamed files and folders formerly known as
agent*torunner*.Review Recommendation: There were quite a lot of files touched and/or removed in this PR, so I would recommend starting with a close look at the changes that happened in the host/main file to get an overall sense of how removing the agent process minimally affects the overall control flow for the harness.
TODO