Skip to content

Use highlighter during tests#14261

Merged
seanbudd merged 1 commit intomasterfrom
systest-useHighlighter
Dec 19, 2022
Merged

Use highlighter during tests#14261
seanbudd merged 1 commit intomasterfrom
systest-useHighlighter

Conversation

@feerrenrut
Copy link
Copy Markdown
Contributor

Link to issue number:

Splitting up PR #14054

Summary of the issue:

When a system test fails a screenshot is captured.
However, this does not make it clear where the focus/ nav / virtual cursor is positioned.

Description of user facing changes

None

Description of development approach

Seeing focus / nav / virtual cursor during tests can help some developers with debugging issues with the tests.

Testing strategy:

Used locally.

Known issues with pull request:

May increase the time system tests take.

Change log entries:

None

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

Seeing focus / nav / browsemode during tests can help some
developers with debugging issues with the tests.
@feerrenrut
Copy link
Copy Markdown
Contributor Author

Given concerns about how this change affects timing, I intend to merge this PR after the other system test PRs

@CyrilleB79
Copy link
Copy Markdown
Contributor

Some questions on this PR:

  1. Why do you have concerns about test timings with this feature?
  2. Why don't you enable visual highlighters in the two other configs? Useful in case the tests using them fail.

@feerrenrut
Copy link
Copy Markdown
Contributor Author

  1. Why don't you enable visual highlighters in the two other configs? Useful in case the tests using them fail.

Happy for this to happen. I just started with the most commonly used config.

  1. Why do you have concerns about test timings with this feature?
  • From casual observation, NVDA is less responsive when using NVDA highlighter.
  • I have a theory that contention for execution context on the main thread causes extra overhead for the checking conditions. I found that a low interval value for _blockUntilConditionMet starved the core queue processing. The outcome of this is that the conditions take longer to be met, perhaps not being met before the time limit. This can add seconds onto every condition test, the impact of many condition checks per test, over many tests should add up to a statistically significant change in total test execution time. Merging this change last (and after a number of builds of the other changes have already occurred), allows first establishing a baseline for the execution time. The initial investigation/prototype PR shows some changes to _blockUntilConditionMet to improve this, additionally the strategy for collecting speech has changed. Those changes will be explained further in another PR.

@feerrenrut feerrenrut marked this pull request as ready for review December 8, 2022 09:17
@feerrenrut feerrenrut requested a review from a team as a code owner December 8, 2022 09:17
@seanbudd seanbudd merged commit c3cdcee into master Dec 19, 2022
@seanbudd seanbudd deleted the systest-useHighlighter branch December 19, 2022 00:01
@nvaccessAuto nvaccessAuto added this to the 2023.1 milestone Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants