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.

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

configurable timeout and delay values #61

Merged
merged 8 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
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]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[string] means "an array with exactly one string element"; what we want here is

Suggested change
* @returns [string]
* @returns {string[]}

(The compiler isn't flagging this right now, presumably due to the absence of curly braces. I guess it considers that "not JSDoc" rather than "invalid JSDoc".)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops! thanks! (still not super used to the jsdoc format)

*/
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).
*/
Loading