Adding waitForTimeout/waitForSelector for assets discovery browser when JS is enabled #1715
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amandeepsingh333
requested review from
ninadbstack and
shantanuk-browserstack
amandeepsingh333
force-pushed
the
PPLT_3191_config_changes
branch
4 times, most recently
from
6a71cc4
to
9dd0779
Compare
amandeepsingh333
force-pushed
the
PPLT_3191_config_changes
branch
from
9dd0779
to
24b65c8
Compare
|
amandeepsingh333
changed the title
amandeepsingh333
changed the title
amandeepsingh333
force-pushed
the
PPLT_3191_config_changes
branch
2 times, most recently
from
d21f946
to
7c530ec
Compare
amandeepsingh333
changed the title
amandeepsingh333
force-pushed
the
PPLT_3191_config_changes
branch
from
7c530ec
to
a710f44
Compare
|
ninadbstack
reviewed
Sep 12, 2024
amandeepsingh333
force-pushed
the
PPLT_3191_config_changes
branch
from
a710f44
to
3f4b118
Compare
amandeepsingh333
changed the title
ninadbstack
approved these changes
Sep 12, 2024
amandeepsingh333
force-pushed
the
PPLT_3191_config_changes
branch
3 times, most recently
from
aca9bea
to
a2c68fc
Compare
packages/core/test/discovery.test.js
Outdated
Comment on lines
2498
to
2506
| it('waitForSelectorInsideBrowser should work', async () => { | ||
| const page = await percy.browser.page(); | ||
| spyOn(page, 'eval').and.callThrough(); | ||
| waitForSelectorInsideBrowser(page, 'body', 30000); | ||
|
|
||
| await percy.idle(); | ||
|
|
||
| expect(page.eval).toHaveBeenCalledWith('await waitForSelector("body", 30000)'); | ||
| }); |
There was a problem hiding this comment.
Move this test inside utils.test.js, as you are directly testing a function of utils.js
packages/core/test/discovery.test.js
Outdated
| '[percy:core:discovery] Wait for 100ms timeout' | ||
| ])); | ||
| }); | ||
| it('waitForTimeout and waitForSelector are not called Js is disabled and domSnapshot is not present', async () => { |
There was a problem hiding this comment.
describe('when JS is enabled')
it('calls page.eval when selector is given')
- Check if page.eval is called for selector
describe('when JS is disabled')
it('not calls page.eval')
There was a problem hiding this comment.
Rest everything looks fine to me
this-is-shivamsingh
approved these changes
Sep 13, 2024
amandeepsingh333
force-pushed
the
PPLT_3191_config_changes
branch
from
a2c68fc
to
e728df3
Compare
amandeepsingh333
force-pushed
the
PPLT_3191_config_changes
branch
from
e728df3
to
51ecf8c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
so now waitForTimeout and waitForSelector are at 2 places:
usage of both:
when it is passed at snapshot level =>
these options are used before a DOM snapshot is taken. The primary goal is to ensure that the page has fully loaded and any necessary DOM elements are present before capturing the state of the page. This mode is currently only supported when used with percy snapshot cli command.
when it is passed at discovery level and JS is enabled => in this phase, these options are used to wait for specific assets or network requests to be discovered and captured. The asset-discovery process often occurs during or after initial page load, and some resources may not be immediately available.
how client should use:
snapshot level => You should use these options at the snapshot level when you want to ensure the readiness of a page before it is captured. This ensures that no partial or incomplete content is included in the snapshot.
discovery level => You should use these options in the discovery level when you are facing issues with missing assets or incomplete network requests. By waiting for the selector or a set timeout, you give enough time for assets to load