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

Change the way we load the page for LCP #72

Closed
Miraeld opened this issue Apr 29, 2024 · 2 comments · Fixed by #75
Closed

Change the way we load the page for LCP #72

Miraeld opened this issue Apr 29, 2024 · 2 comments · Fixed by #75
Assignees
Labels
enhancement New feature or request

Comments

@Miraeld
Copy link
Contributor

Miraeld commented Apr 29, 2024

Is your feature request related to a problem? Please describe.

It's not related to a problem, but more like an enhancement.
Right now, in the LCP tests, we are loading the page, and wait for 2 seconds before closing it (here).
I was thinking, as we implemented this for the SaaS to know when the script has finished to run if we could re-use it for the e2e.
As it adds an attribute to an element for the SaaS to detect the end of the script, we could re-use it to detect it on the e2e process as well.

Describe the solution you'd like

Detect the attributes the same way the SaaS does so we can avoid waiting for a period of time which could be not enough OR too long compare to the real time needed for the script to run.

Describe alternatives you've considered

Additional context

@Miraeld Miraeld added the enhancement New feature or request label Apr 29, 2024
@Miraeld
Copy link
Contributor Author

Miraeld commented May 6, 2024

Scope a solution
To implement this feature, we must change one line:

await this.page.waitForTimeout(2000);

to become:

await this.page.waitForFunction(
      'document.querySelector(\'[data-name="wpr-lcp-beacon"]\').getAttribute(\'beacon-completed\') === \'true\''
);

Development steps:

Effort estimation: XS

@Miraeld Miraeld self-assigned this May 6, 2024
@jeawhanlee
Copy link
Contributor

Instead of wrapping in a string we can define this in a arrow function to make the code better and easier to understand:
Maybe something like this:

await this.page.waitForFunction(() => {
    const beacon = document.querySelector('[data-name="wpr-lcp-beacon"]');
    return beacon && beacon.getAttribute('beacon-completed') === 'true';
});

Aside that LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants