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

test: minimize interchange debouce test race condition #11259

Conversation

ncoden
Copy link
Contributor

@ncoden ncoden commented May 13, 2018

Description

This pull request prevent some race condition by improving the interchange debounce test precision caused by an asynchronous usage of timeouts. Timeout delays are most often not respected and the differences between several timeouts running in parrallel can be huge.

Partially replaces #11214

Changes:

Note about the race condition:

I don't think it is possible to fully remove any race condition in this test. We want to test a behavior within a 10ms delay but we have no certainty that the used timeout will be triggered when expected and not latter.

We can increase the debounce time used in the test to ensure that all the "resize" will be triggered within the debounce interval.

An other solution could be to trigger a huge number of "resize" events and test if _reflow got called up to one time per debounce interval. But how to be synchronised with these debounce interval, and could this make the test way too specific ?

Note about the commit convention:

I was not sure before wether of the fix or test prefix we should use for this "test fix". According to the Angular commit convention it should be test:

test: Adding missing tests or correcting existing tests

Types of changes

  • Documentation
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing
    functionality to change)

Checklist (all required):

  • I have read and follow the CONTRIBUTING document.
  • There are no other pull request similar to this one.
  • The pull request title is descriptive.
  • The template is fully and correctly filled.
  • The pull request targets the right branch (develop or support/*).
  • My commits are correctly titled and contain all relevant information.
  • My code follows the code style of this project.
  • I have updated the documentation accordingly to my changes (if relevant).
  • I have added tests to cover my changes (if relevant).
  • All new and existing tests passed.

⚠️ Require:

ncoden added 2 commits May 13, 2018 21:38
Changes:
* check if Triggers is already initialized just before it should be initialized, once the window is loaded.
Prevent some race condition by improving the interchange debounce test precision.

Changes:
* initialize Triggers manually to control and test the debounce time
* nest the timeout that trigger "resize" to make the delay between the last "resize" and the test check more precise (a bit more than the debounce time).

Timeout delays are most often not respected and the differences between several timeouts running in parrallel can be huge.
@ncoden ncoden merged commit eba8f38 into foundation:develop May 15, 2018
ncoden added a commit to ncoden/foundation-sites that referenced this pull request Jun 16, 2018
…ge-debouce-test-race-condition for v6.5.0

4419192 fix: prevent to initialize Triggers twice before window is loaded
0e72c1e test: improve Interchange debounce test precision

Signed-off-by: Nicolas Coden <[email protected]>
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.

2 participants