configurable timeout and delay values (w3c#61)

* 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

Author: gnarf

src/agent/browser-driver/create-safari-apple-script-driver.js

@@ -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>}
@@ -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
@@ -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;

src/agent/browser-driver/create.js

@@ -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;

src/agent/cli.js

@@ -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) {
@@ -76,6 +77,7 @@ export function buildAgentCliOptions(args = yargs) {
         choices: ['request', 'skip'],
         hidden: true,
       },
+      ...timesOptionsConfig,
     })
     .showHidden('show-hidden');
 }
@@ -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}`);
     }
@@ -149,6 +154,7 @@ export function pickAgentCliOptions({
   atDriverUrl,
   mock,
   mockOpenPage,
+  timesOption,
 }) {
   return {
     ...(debug === undefined ? {} : { debug }),
@@ -160,6 +166,7 @@ export function pickAgentCliOptions({
     ...(atDriverUrl === undefined ? {} : { atDriverUrl }),
     ...(mock === undefined ? {} : { mock }),
     ...(mockOpenPage === undefined ? {} : { mockOpenPage }),
+    timesOption,
   };
 }
 
@@ -245,6 +252,7 @@ async function agentRunnerMiddleware(argv) {
     webDriverBrowser: argv.webDriverBrowser,
     atDriverUrl: argv.atDriverUrl,
     abortSignal: argv.abortSignal,
+    timesOption: getTimesOption(argv),
   });
 }
 

src/agent/create-test-runner.js

@@ -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>}
  */
@@ -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 });
     }),
@@ -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 });
 }

src/agent/driver-test-runner.js

@@ -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() {
@@ -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 });
@@ -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) {
@@ -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,
@@ -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');
         });

src/agent/types.js

@@ -57,6 +57,7 @@
  * @property {AriaATCIShared.BaseURL} [webDriverUrl]
  * @property {AriaATCIAgent.Browser} [webDriverBrowser]
  * @property {AriaATCIShared.BaseURL} [atDriverUrl]
+ * @property {AriaATCIShared.timesOption} [timesOption]
  */
 
 /**

src/host/agent.js

@@ -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'),

src/host/cli-run-plan.js

@@ -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..]';
 
@@ -156,6 +157,7 @@ export const builder = (args = yargs) =>
           return { [name]: value };
         },
       },
+      ...timesOptionsConfig,
     })
     .showHidden('show-hidden')
     .middleware(verboseMiddleware)
@@ -272,6 +274,8 @@ function mainAgentMiddleware(argv) {
     agentMockOpenPage,
   } = argv;
 
+  const timesOption = getTimesOption(argv);
+
   argv.agent = new Agent({
     log,
     protocol,
@@ -284,6 +288,7 @@ function mainAgentMiddleware(argv) {
       atDriverUrl: agentAtDriverUrl,
       mock: agentMock,
       mockOpenPage: agentMockOpenPage,
+      timesOption: timesOption,
     }),
   });
 }

src/shared/times-option.js

@@ -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;
+}

src/shared/types.js

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