Skip to content

Lower poll interval for blockUntilConditionMet#14324

Merged
seanbudd merged 1 commit into
masterfrom
lowerPollInterval
Dec 7, 2022
Merged

Lower poll interval for blockUntilConditionMet#14324
seanbudd merged 1 commit into
masterfrom
lowerPollInterval

Conversation

@seanbudd

@seanbudd seanbudd commented Nov 2, 2022

Copy link
Copy Markdown
Member

Link to issue number:

None

Summary of the issue:

Due to intermittent system test failures, #14284 increased the polling interval used in blockUntilConditionMet.
When writing unit tests for blockUntilConditionMet in #14301, a bug was picked up. This bug caused blockUntilConditionMet to spin for longer than expected and potentially caused system tests to fail.

This bug was fixed with a new implementation of blockUntilConditionMet.

Description of user facing changes

For devs, system tests should be faster (todo: estimate from build?)

Description of development approach

Lower default polling interval

Testing strategy:

Monitor system tests for a while, check if blockUntilConditionMet causes a system test failure.

Known issues with pull request:

None

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.

@seanbudd seanbudd requested a review from a team as a code owner November 2, 2022 00:13
@seanbudd seanbudd requested a review from feerrenrut November 2, 2022 00:13
@seanbudd seanbudd mentioned this pull request Nov 2, 2022
@seanbudd seanbudd self-assigned this Nov 2, 2022

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change looks good, but I'd like to know if system tests on master are stable before we merge this. It looks like it reduces the test time from 26.7 min to 23.3 min (only based on one run of each branch).

@seanbudd seanbudd merged commit 53723b0 into master Dec 7, 2022
@seanbudd seanbudd deleted the lowerPollInterval branch December 7, 2022 05:30
@nvaccessAuto nvaccessAuto added this to the 2023.1 milestone Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants