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

Improve Facebook Click to Load integration tests #1039

Merged
merged 1 commit into from
Feb 9, 2022

Conversation

kzar
Copy link
Collaborator

@kzar kzar commented Jan 28, 2022

Improve Facebook Click to Load integration tests

Improvements to the Facebook Click to Load integration tests,
including:

  1. Reduce flaky failures by waiting until all the required extension
    configuration has been loaded.
  2. Log requests using Puppeteer instead of relying on the test page
    to accurately report the number of allowed/blocked requests.
  3. Test requests to the SDK are redirected properly.
  4. Test that unblocking works correctly and that reloading the page
    restores blocking.

This change also:

  • Renames and expands on the background waiting integration test
    helper module. That way, we can avoid duplicating configuration
    waiting logic.
  • Tweaks the request logging helper module to avoid logging in-flight
    requests that started before a navigation but finished
    afterwards. That race condition was the cause of some test flakiness.
  • Tweaks the YouTube CTL tests to wait for "networkidle0" instead of
    "networkidle2". That means waiting for all requests to complete,
    rather than all but two.

Reviewer: @jonathanKingston

CC: @ladamski, @Charlie-belmer, @sammacbeth, @kdzwinel
Depends on: #978

Description:

This change makes use of the YouTube Click to Load work to improve the Facebook Click to Load integration tests.

Steps to test this PR:

  1. Run the integration tests (to do that locally type npm run test-int.

Automated tests:

  • Unit tests
  • Integration tests
Reviewer Checklist:
  • Ensure the PR solves the problem
  • Review every line of code
  • Ensure the PR does no harm by testing the changes thoroughly
  • Get help if you're uncomfortable with any of the above!
  • Determine if there are any quick wins that improve the implementation
PR Author Checklist:
  • Get advice or leverage existing code
  • Agree on technical approach with reviewer (if the changes are nuanced)
  • Ensure that there is a testing strategy (and documented non-automated tests)
  • Ensure there is a documented monitoring strategy (if necessary)
  • Consider systems implications

@kzar kzar force-pushed the fb-clicktoplay-improvetests branch from bf42fdf to caced37 Compare February 3, 2022 17:10
@kzar kzar marked this pull request as ready for review February 3, 2022 17:11
Copy link
Member

@kdzwinel kdzwinel left a comment

Choose a reason for hiding this comment

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

Nicely done! I left some comments, mostly nits 👀

@kzar kzar force-pushed the fb-clicktoplay-improvetests branch 2 times, most recently from 28f40ea to f77d182 Compare February 4, 2022 15:51
@kzar kzar force-pushed the fb-clicktoplay-improvetests branch 7 times, most recently from 74258d3 to cd23f06 Compare February 8, 2022 17:58
Improvements to the Facebook Click to Load integration tests,
including:

 1. Reduce flaky failures by waiting until all the required extension
    configuration has been loaded.
 2. Log requests using Puppeteer instead of relying on the test page
    to accurately report the number of allowed/blocked requests.
 3. Test requests to the SDK are redirected properly.
 4. Test that unblocking works correctly and that reloading the page
    restores blocking.

This change also:

 - Renames and expands on the background waiting integration test
   helper module. That way, we can avoid duplicating configuration
   waiting logic.
 - Tweaks the request logging helper module to avoid logging in-flight
   requests that started before a navigation but finished
   afterwards. That race condition was the cause of some test flakiness.
 - Tweaks the YouTube CTL tests to wait for "networkidle0" instead of
   "networkidle2". That means waiting for all requests to complete,
   rather than all but two.
@kzar kzar force-pushed the fb-clicktoplay-improvetests branch from cd23f06 to 30a4908 Compare February 8, 2022 18:02
@kdzwinel
Copy link
Member

kdzwinel commented Feb 9, 2022

Thanks! LGTM.

@kdzwinel kdzwinel merged commit 1308e91 into duckduckgo:develop Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants