Skip to content

Commit

Permalink
configurable timeout and delay values (#61)
Browse files Browse the repository at this point in the history
* configurable timeout and delay values

* types draft

* individual options for each time

* more restrictive types for the helper functions

* prefer lowercase string and number type (seems to be tsc standard)

* remove makeSnakeCasedKey function

* pass timesOption config to modules instead of stateful module

* fix jsdoc
  • Loading branch information
gnarf authored Jul 23, 2024
1 parent 21230f6 commit 9ef800c
Show file tree
Hide file tree
Showing 10 changed files with 153 additions and 23 deletions.
10 changes: 5 additions & 5 deletions src/agent/browser-driver/create-safari-apple-script-driver.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { execFile } from 'child_process';

/** Number of milliseconds to wait for document to be ready before giving up. */
const DOCUMENT_READY_TIMEOUT = 2000;

/**
* @param {string} source
* @returns {Promise<string>}
Expand Down Expand Up @@ -43,7 +40,10 @@ const evalJavaScript = source => {
end tell`);
};

export default async () => {
/**
* @param {AriaATCIShared.timesOption} timesOption
*/
export default async timesOption => {
await execScript(`tell application "Safari"
if documents = {} then make new document
activate
Expand All @@ -60,7 +60,7 @@ export default async () => {
async documentReady() {
const start = Date.now();

while (Date.now() - start < DOCUMENT_READY_TIMEOUT) {
while (Date.now() - start < timesOption.docReady) {
const readyState = await evalJavaScript('document.readyState');
if (readyState === 'complete') {
return;
Expand Down
5 changes: 3 additions & 2 deletions src/agent/browser-driver/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ import createSafariAppleScriptDriver from './create-safari-apple-script-driver.j
* @param {{toString: function(): string}} options.url
* @param {AriaATCIAgent.Browser} [options.browser]
* @param {Promise<void>} options.abortSignal
* @param {AriaATCIShared.timesOption} options.timesOption
*
* @returns {Promise<BrowserDriver>}
*/
export async function createBrowserDriver({ url, browser = 'firefox', abortSignal }) {
export async function createBrowserDriver({ url, browser = 'firefox', abortSignal, timesOption }) {
const driver =
browser === 'safari'
? await createSafariAppleScriptDriver()
? await createSafariAppleScriptDriver(timesOption)
: await createWebDriver(browser, url.toString());
abortSignal.then(() => driver.quit());
return driver;
Expand Down
8 changes: 8 additions & 0 deletions src/agent/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { iterateEmitter } from '../shared/iterate-emitter.js';
import { createRunner } from './create-test-runner.js';
import { agentMain } from './main.js';
import { AgentMessage, createAgentLogger } from './messages.js';
import { getTimesOption, timesArgs, timesOptionsConfig } from '../shared/times-option.js';

/** @param {yargs} args */
export function buildAgentCliOptions(args = yargs) {
Expand Down Expand Up @@ -76,6 +77,7 @@ export function buildAgentCliOptions(args = yargs) {
choices: ['request', 'skip'],
hidden: true,
},
...timesOptionsConfig,
})
.showHidden('show-hidden');
}
Expand Down Expand Up @@ -128,6 +130,9 @@ export function agentCliArgsFromOptionsMap(options) {
case 'mockOpenPage':
args.push(`--mock-open-page=${value}`);
break;
case 'timesOption':
args.push(...timesArgs(value));
break;
default:
throw new Error(`unknown agent cli argument ${key}`);
}
Expand All @@ -149,6 +154,7 @@ export function pickAgentCliOptions({
atDriverUrl,
mock,
mockOpenPage,
timesOption,
}) {
return {
...(debug === undefined ? {} : { debug }),
Expand All @@ -160,6 +166,7 @@ export function pickAgentCliOptions({
...(atDriverUrl === undefined ? {} : { atDriverUrl }),
...(mock === undefined ? {} : { mock }),
...(mockOpenPage === undefined ? {} : { mockOpenPage }),
timesOption,
};
}

Expand Down Expand Up @@ -245,6 +252,7 @@ async function agentRunnerMiddleware(argv) {
webDriverBrowser: argv.webDriverBrowser,
atDriverUrl: argv.atDriverUrl,
abortSignal: argv.abortSignal,
timesOption: getTimesOption(argv),
});
}

Expand Down
5 changes: 4 additions & 1 deletion src/agent/create-test-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { AgentMessage } from './messages.js';
* @param {AriaATCIAgent.Log} options.log
* @param {AriaATCIAgent.MockOptions} [options.mock]
* @param {AriaATCIAgent.Browser} [options.webDriverBrowser]
* @param {AriaATCIShared.timesOption} options.timesOption
* @param {{toString: function(): string}} options.webDriverUrl
* @returns {Promise<AriaATCIAgent.TestRunner>}
*/
Expand All @@ -30,11 +31,13 @@ export async function createRunner(options) {
return new MockTestRunner({ mock: options.mock, ...options });
}
await new Promise(resolve => setTimeout(resolve, 1000));
const { timesOption } = options;
const [browserDriver, atDriver] = await Promise.all([
createBrowserDriver({
url: options.webDriverUrl,
browser: options.webDriverBrowser,
abortSignal: options.abortSignal,
timesOption,
}).catch(cause => {
throw new Error('Error initializing browser driver', { cause });
}),
Expand All @@ -46,5 +49,5 @@ export async function createRunner(options) {
throw new Error('Error connecting to at-driver', { cause });
}),
]);
return new DriverTestRunner({ ...options, browserDriver, atDriver });
return new DriverTestRunner({ ...options, browserDriver, atDriver, timesOption });
}
23 changes: 8 additions & 15 deletions src/agent/driver-test-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,22 @@ import { AgentMessage } from './messages.js';
* @module agent
*/

const AFTER_NAVIGATION_DELAY = 1000;
const AFTER_KEYS_DELAY = 5000;
const RUN_TEST_SETUP_BUTTON_TIMEOUT = 1000;

export class DriverTestRunner {
/**
* @param {object} options
* @param {AriaATCIShared.BaseURL} options.baseUrl
* @param {AriaATCIAgent.Log} options.log
* @param {BrowserDriver} options.browserDriver
* @param {ATDriver} options.atDriver
* @param {AriaATCIShared.timesOption} options.timesOption
*/
constructor({ baseUrl, log, browserDriver, atDriver }) {
constructor({ baseUrl, log, browserDriver, atDriver, timesOption }) {
this.baseUrl = baseUrl;
this.log = log;
this.browserDriver = browserDriver;
this.atDriver = atDriver;
this.collectedCapabilities = this.getCapabilities();
this.timesOption = timesOption;
}

async getCapabilities() {
Expand All @@ -51,7 +49,7 @@ export class DriverTestRunner {
try {
await this.browserDriver.clickWhenPresent(
'.button-run-test-setup',
RUN_TEST_SETUP_BUTTON_TIMEOUT
this.timesOption.testSetup
);
} catch ({}) {
await this.log(AgentMessage.NO_RUN_TEST_SETUP, { referencePage });
Expand All @@ -71,15 +69,10 @@ export class DriverTestRunner {
* @param {string} desiredResponse
*/
async pressKeysToToggleSetting(sequence, desiredResponse) {
// This timeout may be reached as many as two times for every test.
// Delays of over 500ms have been observed during local testing in a
// Windows virtual machine.
const MODE_SWITCH_SPEECH_TIMEOUT = 750;

let unknownCollected = '';
// there are 2 modes, so we will try pressing mode switch up to twice
for (let triesRemain = 2; triesRemain > 0; triesRemain--) {
const speechResponse = await this._collectSpeech(MODE_SWITCH_SPEECH_TIMEOUT, () =>
const speechResponse = await this._collectSpeech(this.timesOption.modeSwitch, () =>
this.sendKeys(sequence)
);
while (speechResponse.length) {
Expand Down Expand Up @@ -202,7 +195,7 @@ export class DriverTestRunner {
const { value: validCommand, errors } = validateKeysFromCommand(command);

if (validCommand) {
await this._collectSpeech(AFTER_NAVIGATION_DELAY, () =>
await this._collectSpeech(this.timesOption.afterNav, () =>
this.openPage({
url: this._appendBaseUrl(test.target.referencePage),
referencePage: test.target.referencePage,
Expand All @@ -217,11 +210,11 @@ export class DriverTestRunner {
await this.ensureMode(test.target.mode);
}

const spokenOutput = await this._collectSpeech(AFTER_KEYS_DELAY, () =>
const spokenOutput = await this._collectSpeech(this.timesOption.afterKeys, () =>
this.sendKeys(atKeysFromCommand(validCommand))
);

await this._collectSpeech(AFTER_NAVIGATION_DELAY, async () => {
await this._collectSpeech(this.timesOption.afterNav, async () => {
await this.log(AgentMessage.OPEN_PAGE, { url: 'about:blank' });
await this.browserDriver.navigate('about:blank');
});
Expand Down
1 change: 1 addition & 0 deletions src/agent/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
* @property {AriaATCIShared.BaseURL} [webDriverUrl]
* @property {AriaATCIAgent.Browser} [webDriverBrowser]
* @property {AriaATCIShared.BaseURL} [atDriverUrl]
* @property {AriaATCIShared.timesOption} [timesOption]
*/

/**
Expand Down
1 change: 1 addition & 0 deletions src/host/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ class AgentDeveloperProtocol extends AgentProtocol {
atDriverUrl: options.atDriverUrl,
webDriverBrowser: options.webDriverBrowser,
webDriverUrl: options.webDriverUrl,
timesOption: options.timesOption,
}),
log,
tests: iterateEmitter(this._testEmitter, 'message', 'stop'),
Expand Down
5 changes: 5 additions & 0 deletions src/host/cli-run-plan.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { hostMain } from './main.js';
import { HostMessage, createHostLogger } from './messages.js';
import { plansFrom } from './plan-from.js';
import { HostServer } from './server.js';
import { getTimesOption, timesOptionsConfig } from '../shared/times-option.js';

export const command = 'run-plan [plan-files..]';

Expand Down Expand Up @@ -156,6 +157,7 @@ export const builder = (args = yargs) =>
return { [name]: value };
},
},
...timesOptionsConfig,
})
.showHidden('show-hidden')
.middleware(verboseMiddleware)
Expand Down Expand Up @@ -272,6 +274,8 @@ function mainAgentMiddleware(argv) {
agentMockOpenPage,
} = argv;

const timesOption = getTimesOption(argv);

argv.agent = new Agent({
log,
protocol,
Expand All @@ -284,6 +288,7 @@ function mainAgentMiddleware(argv) {
atDriverUrl: agentAtDriverUrl,
mock: agentMock,
mockOpenPage: agentMockOpenPage,
timesOption: timesOption,
}),
});
}
Expand Down
109 changes: 109 additions & 0 deletions src/shared/times-option.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/// <reference path="./types.js" />

/**
* @module shared
*/

/**
* @type AriaATCIShared.timesOption
*/
const timesDefaults = {
afterNav: 1000,
afterKeys: 5000,
testSetup: 1000,
modeSwitch: 750,
docReady: 2000,
};

/**
* Create a yargs description for the specified timesOption.
* @param {keyof AriaATCIShared.timesOption} optionName Key from timesOption
* @param {string} argName The text used for the argument (without leading --)
* @param {string} describe Description to be used in --show-help
*/
function addOptionConfig(optionName, argName, describe) {
timesOptionsArgNameMap.set(optionName, argName);
timesOptionsConfig[argName] = {
hidden: true,
default: timesDefaults[optionName],
describe,
coerce(arg) {
const isNumber = typeof arg === 'number';
if (!isNumber && !arg.match(/^\d+$/)) {
throw new Error('option value not a number');
}
const time = isNumber ? arg : parseInt(arg, 10);
if (time <= 0) {
throw new Error('time must be positive and non-zero');
}
return time;
},
};
}

/**
* @type Map<keyof AriaATCIShared.timesOption, string>
*/
const timesOptionsArgNameMap = new Map();

/**
* the yargs configuration for the time options
*/
export const timesOptionsConfig = {};
addOptionConfig(
'afterNav',
'time-after-nav',
'Timeout used after navigation to collect and discard speech.'
);
addOptionConfig(
'afterKeys',
'time-after-keys',
'Timeout used to wait for speech to finish after pressing keys.'
);
addOptionConfig(
'testSetup',
'time-test-setup',
'Timeout used after pressing test setup button to collect and discard speech.'
);
addOptionConfig(
'modeSwitch',
'time-mode-switch',
'Timeout used after switching modes to check resulting speech (NVDA).'
);
addOptionConfig('docReady', 'time-doc-ready', 'Timeout used waiting for document ready (Safari).');

/**
* Convert the times dictionary to an array of strings to pass back to args.
* @param {AriaATCIShared.timesOption} opts
* @returns {string[]}
*/
export function timesArgs(opts) {
const args = [];
for (const key of Object.keys(opts)) {
const value = opts[key];
// no need to pass on "default" value
if (value == timesDefaults[key]) continue;
// casting in jsdoc syntax is complicated - the extra () around key are
// required to make the type annotation work.
const argName = timesOptionsArgNameMap.get(/** @type keyof AriaATCIShared.timesOption */ (key));
args.push('--' + argName);
args.push(String(value));
}
return args;
}

/**
* Convert the arguments parse result into a timesOption object.
* @param {any} args The parsed arguments
* @returns {AriaATCIShared.timesOption}
*/
export function getTimesOption(args) {
const result = { ...timesDefaults };
for (const key in result) {
const mapped = timesOptionsArgNameMap.get(/** @type keyof AriaATCIShared.timesOption */ (key));
if (mapped) {
if (args[mapped]) result[key] = args[mapped];
}
}
return result;
}
9 changes: 9 additions & 0 deletions src/shared/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,12 @@
* @param {AriaATCIShared.JobBinding<*>} binding
* @returns {Promise<T>}
*/

/**
* @typedef AriaATCIShared.timesOption
* @property {number} afterNav Timeout used after navigation to collect and discard speech.
* @property {number} afterKeys Timeout used to wait for speech to finish after pressing keys.
* @property {number} testSetup Timeout used after pressing test setup button to collect and discard speech.
* @property {number} modeSwitch Timeout used after switching modes to check resulting speech (NVDA).
* @property {number} docReady Timeout used waiting for document ready (Safari).
*/

0 comments on commit 9ef800c

Please sign in to comment.