configurable timeout and delay values #61
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,91 @@ | ||||||
| /// <reference path="./types.js" /> | ||||||
|
|
||||||
| /** | ||||||
| * @module shared | ||||||
| */ | ||||||
|
|
||||||
| /** | ||||||
| * @type AriaATCIShared.timesOption | ||||||
| */ | ||||||
| export const timesOption = { | ||||||
| afterNav: 1000, | ||||||
| afterKeys: 5000, | ||||||
| testSetup: 1000, | ||||||
| modeSwitch: 750, | ||||||
| docReady: 2000, | ||||||
| }; | ||||||
|
|
||||||
| /** | ||||||
| * @type AriaATCIShared.timesOption | ||||||
| */ | ||||||
| const timesDefaults = { ...timesOption }; | ||||||
|
|
||||||
| /** | ||||||
| * Convert from 'afterNav' to 'time-after-nav'. | ||||||
| * @param {keyof AriaATCIShared.timesOption} optionName | ||||||
| * @returns string | ||||||
| */ | ||||||
| function makeSnakeCasedOption(optionName) { | ||||||
| const snakeCased = optionName.replace(/[A-Z]/g, cap => '-' + cap.toLowerCase()); | ||||||
| const optionText = `time-${snakeCased}`; | ||||||
| return optionText; | ||||||
| } | ||||||
|
There was a problem hiding this comment. I kinda have a thing for grep-able source code. I'd much rather maintain a bit of redundancy (e.g. an argument list like There was a problem hiding this comment. pushed this change in |
||||||
|
|
||||||
| /** | ||||||
| * Create a yargs description for the specified timesOption. | ||||||
| * @param {keyof AriaATCIShared.timesOption} optionName Key from timesOption | ||||||
| * @param {string} describe Description to be used in --show-help | ||||||
| */ | ||||||
| function addOptionConfig(optionName, describe) { | ||||||
| timesOptionsConfig[makeSnakeCasedOption(optionName)] = { | ||||||
| hidden: true, | ||||||
| default: timesOption[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'); | ||||||
| } | ||||||
| timesOption[optionName] = time; | ||||||
| }, | ||||||
| }; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * the yargs configuration for the time options | ||||||
| */ | ||||||
| export const timesOptionsConfig = {}; | ||||||
| addOptionConfig('afterNav', 'Timeout used after navigation to collect and discard speech.'); | ||||||
| addOptionConfig('afterKeys', 'Timeout used to wait for speech to finish after pressing keys.'); | ||||||
| addOptionConfig( | ||||||
| 'testSetup', | ||||||
| 'Timeout used after pressing test setup button to collect and discard speech.' | ||||||
| ); | ||||||
| addOptionConfig( | ||||||
| 'modeSwitch', | ||||||
| 'Timeout used after switching modes to check resulting speech (NVDA).' | ||||||
| ); | ||||||
| addOptionConfig('docReady', '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] | ||||||
|
There was a problem hiding this comment.
Suggested change
(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".) There was a problem hiding this comment. oops! thanks! (still not super used to the jsdoc format) |
||||||
| */ | ||||||
| export function timesArgs(opts = timesOption) { | ||||||
| const args = []; | ||||||
| for (const key of Object.keys(timesOption)) { | ||||||
| const value = timesOption[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. | ||||||
| args.push(makeSnakeCasedOption(/** @type {keyof AriaATCIShared.timesOption} */ (key))); | ||||||
| args.push(String(value)); | ||||||
| } | ||||||
| return args; | ||||||
| } | ||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stateful modules are a source of tech debt in my experience. This clearly works for our current needs, but at some point in the future (e.g. when we go to write a new kind of test or re-use something), the
timesOptionsingleton may make the change more involved than if we had explicitly passed the value around to begin with. Think we can treat these values a little more like the rest of the command-line arguments?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible, the reason I was sort of avoiding it is this would necessitate passing these options into modules everywhere, I feel like the usability/dev experience of adding a timeout to it in the future might be pretty large. I'll do this as a separate commit to kind of weigh it out (not having done it yet, it might not be "that bad")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the end this didn't feel too terrible. ea8d351