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

Use fraggle rock API for conducting lighthouse tests #44

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

asambstack
Copy link

@asambstack asambstack commented Feb 15, 2023

In the legacy lighthouse flow, a browser needs to be started with remote debugging port enabled and lighthouse starts testing by firing CDP commands over this RDP. Lighthouse team has been working on the fraggle rock API with flow support and several API changes (see: GoogleChrome/lighthouse#11313). This also includes a navigation API that can connect to existing puppeteer page instance for performing lighthouse tests.
By modifying the playwright object slightly, we can utilise the fraggle rock API for performing lighthouse tests.

Pros

  • Using existing page object allows for connection with remote browsers. Helpful for running tests on remote browsers of cloud providers such as Browserstack.
  • Since tests will be run in the same page context passed to playwright-lighthouse, authenticated workflows will not break.

Changes

  • Using fraggle rock API for lighthouse tests
  • Added patchPageObject method for patching playwright page
  • Removed redundant cdpPort param, moved util methods to util.js
  • Updated readme with new API

Copy link
Owner

@abhinaba-ghosh abhinaba-ghosh left a comment

Choose a reason for hiding this comment

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

This looks promising.

Could you attach documentation and relevant examples to understand the changes?

@asambstack
Copy link
Author

Sure, was currently updating the readme with examples. Will also attach relevant links that were used as references in this PR.
Currently, lighthouse team themselves haven't created docs for the same. However details of the fraggle rock implementation can be found in https://docs.google.com/document/d/1fRCh_NVK82YmIi1Zq8y73_p79d-FdnKsvaxMy0xIpNw/edit?pli=1#heading=h.fpighughqndf

Copy link

@Archish27 Archish27 left a comment

Choose a reason for hiding this comment

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

woahh nice 🚀

@asambstack
Copy link
Author

Hey @abhinaba-ghosh
Have updated the tests, README and type definition. Could you review it once?

@asambstack asambstack changed the title [WIP] Use fraggle rock API for conducting lighthouse tests Use fraggle rock API for conducting lighthouse tests Feb 21, 2023
@abhinaba-ghosh
Copy link
Owner

I will be testing this today and release a new version.

@abhinaba-ghosh
Copy link
Owner

Hey @asambstack sorry for the delay. Could you rebase your branch and resolve the conflicts?

@Xotabu4
Copy link

Xotabu4 commented Mar 30, 2023

@asambstack any updates on this? Really looking to get this feature into package!

@asambstack
Copy link
Author

Hey @abhinaba-ghosh
Looks like in the newer package version we've released lighthouse 10 support. In lighthouse 10, the fraggle rock API feature has been released with the core package and hence we do not need to use the fraggle rock file imports specifically and can directly use the core lighthouse package itself..
However, came across an issue with using this approach with playwright, it was failing on some sites like nykaa.com. On deeper investigation, looks like we might need to do more work for monkey patching playwright page object to seem like that of puppeteer.

The way lighthouse works with a puppeteer page is that it first creates a session object that acts as a wrapper around a puppeteer session. One of the things that lighthouse does with a puppeteer page is to attach event listeners to it (ref). This is possible due to the fact that puppeteer exposes a connection method to the CDP session object it has created (ref) which is an extension of an event emitter. This connection class exposed by puppeteer emits different events whenever the state of the webpage changes, for eg. Target.attachedToTarget / Target.detachedFromTarget.

Even playwright emits the exact same events since these are CDP events, however this connection class doesn’t exist in case of playwright (ref).

Meaning, to make this work with lighthouse, we would need to create a custom adapter connection class that acts as a bridge between playwright and lighthouse (ref).

Will take a look into the feasibility, can discuss if you've any approaches in mind.

@Xotabu4
Copy link

Xotabu4 commented Apr 12, 2023

@asambstack i took the code from this PR

const patchPageObject = (page) => {
  page.target = function () {
    return {
      createCDPSession: async function () {
        const session = await page.context().newCDPSession(page);
        session.connection = () => new events.EventEmitter();
        return session;
      },
    };
  };
};

Tried on my own project - and so far it works fine for me, even with page navigations initiated by user and webrtc trafic on the pages.

Will keep you updated in case will see some issues

);
const newValues = Object.keys(results.lhr.categories).reduce(
let results;
if (process.env.LH_BROWSERSTACK == 'true') {
Copy link
Owner

Choose a reason for hiding this comment

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

Would you mind updating the readme with this addition? Thanks

@abhinaba-ghosh
Copy link
Owner

@asambstack apologies for a delayed response. Are we still considering moving forward with this PR?

@Xotabu4
Copy link

Xotabu4 commented Mar 1, 2024

@asambstack @abhinaba-ghosh any updates on this? Would be cool to have this merged

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.

4 participants