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

Add tests to lock in the waitUntil lifecycle event behaviour #643

Closed
wants to merge 10 commits into from

Conversation

ankur22
Copy link
Collaborator

@ankur22 ankur22 commented Nov 8, 2022

Before we attempt to cleanup the lifecycle event code (#593) we need to lock in the current behaviour so that any refactoring does not affect the behaviour.

The tests are:

  • test that waitUntil on load and DOMContentLoaded work as expected when used in goto.
  • test that waitUntil on load, DOMContentLoaded and networkidle work as expected when we navigate to a page with a main and sub frame when used in goto.
  • test that waitUntil on load, DOMContentLoaded and networkidle work as expected when used as an option on WaitForNavigation.
  • test that waitUntil on networkidle timesout if we set it too late after the navigation has completed.

Linked issue: #593

This test ensure that we can navigate to a page and wait for the
domcontentloaded lifecycle event. The test ensures that we're not
waiting on the async scripts and other network requests to complete
before we unblock on the wait.
This test will wait until the html and async scripts have loaded
before unblocking the wait. It does not wait for the other network
requests to complete.
This test will ensure that when we navigate to a page with goto and
waitUntil networkidle, where the page is made up of a main and sub
frame, we wait until both frames have completed loading and a
networkidle event has been received.
This test will wait for both the main and sub frames to both have
loaded ONLY the html. It does not wait for the async scripts and
other network requests to complete.
This test will wait for both the main and sub frame to load the html
and the async scripts.
This test locks in the timeout behaviour of WaitForNavigation if it
is called to late once a navigation has already completed. This is
why we suggest that a nav action and WaitForNavigation must be setup
in a promise.All.
The next set of tests requires us to wait on a second set of promises.
This change refactors the code to allow for that.
This test will perform two navigations. The second navigation occurs
after a click action where we use WaitForNavigation to wait until
networkIdle event received.
This test will perform two navigations. The second navigation is due
to a click action. We also setup WaitForNavigation on domcontentloaded
which should succeed in unblocking once the html has loaded. We don't
wait for the async scripts and other network requests in either of the
navigations.
This test will perform two navigations. The second nav occurs after a
click action. At the same time we set WaitForNavigation with load.
The html and async scripts must load before we unblock on both nav
actions.
@ankur22 ankur22 added the tests label Nov 8, 2022
@ankur22 ankur22 added this to the v0.7.0 milestone Nov 8, 2022
@ankur22 ankur22 requested a review from inancgumus November 8, 2022 15:41
@ankur22 ankur22 self-assigned this Nov 8, 2022
@inancgumus inancgumus removed their request for review November 9, 2022 10:23
@ankur22 ankur22 deleted the branch release-v0.7.0 November 10, 2022 13:40
@ankur22 ankur22 closed this Nov 10, 2022
@inancgumus inancgumus deleted the refactor/593-lifecycle-tests branch December 5, 2022 11:37
@inancgumus inancgumus removed this from the v0.7.0 milestone Dec 5, 2022
inancgumus added a commit that referenced this pull request Jan 13, 2023
So far, we haven't been testing the lifecycle tests. The test variables
were captured because `t.Parallel`. This fixes it.

Related: #647, #644, #643
inancgumus added a commit that referenced this pull request Jan 13, 2023
So far, we haven't been testing the lifecycle tests. The test variables
were captured because `t.Parallel`. This fixes it.

Related: #647, #644, #643
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.

2 participants