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

[browser tests] update Puppeteer #12222

Merged
merged 1 commit into from
Feb 27, 2023
Merged

[browser tests] update Puppeteer #12222

merged 1 commit into from
Feb 27, 2023

Conversation

marcdumais-work
Copy link
Contributor

@marcdumais-work marcdumais-work commented Feb 24, 2023

What it does

The version of Puppeteer (and few related dependencies) we had is quite old and uses a version of Chromium that does not support some of the DOM API functions we use in the current code base. e.g. HTMLElement.replaceChildren() [1], that we use in hover-service.ts

The latest version seems to work well, and supports the example above, so I went with that. Note that Puppeteer now bundles its typing file, and that some of the APIs have changed slightly, requiring a small adaptation on our end.

[1]: https://developer.mozilla.org/en-US/docs/Web/API/Element/replaceChildren#browser_compatibility

How to test

  • The browser test should still pass in CI
  • you may also try running them locally: yarn && yarn download:plugins && yarn browser build && yarn browser test
  • you may also confirm that it's still possible to run the suite to gather coverage information:
cd example/browser
yarn -s rebuild && npx theia test . --plugins=local-dir:../../plugins --test-coverage  --test-spec=../api-tests/src/*.spec.js
npx nyc report --reporter=lcov --reporter=text

note: for the code coverage, source-map is not currently configured, so the results will be against the browser app "compiled .js files", not the original Theia sources. Anyway, the immediate goal is just to confirm that the code coverage infrastructure still works after the update.

Review checklist

Reminder for reviewers

@marcdumais-work marcdumais-work marked this pull request as draft February 24, 2023 14:55
@marcdumais-work marcdumais-work changed the title [browser tests] update puppeteer [browser tests] update Puppeteer Feb 24, 2023
@marcdumais-work marcdumais-work marked this pull request as ready for review February 24, 2023 17:25
@marcdumais-work marcdumais-work force-pushed the update-puppeteer branch 2 times, most recently from 6a28f50 to b2f02bf Compare February 24, 2023 17:32
@vince-fugnitto vince-fugnitto added ci issues related to CI / tests playwright issues related to playwright tests labels Feb 27, 2023
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

LGTM 👍

  • CI successfully passes for playwright
  • CI successfully passes for the API integration tests
  • yarn browser test passes locally

The version of Puppeteer we had is quite old and includes a version of
Chromium that does not support some of the DOM API functions we use in the
code base. e.g. HTMLElement.replaceChildren() [1], that we use in
hover-service.ts

The latest version seems to work well, and supports the example above,
so I went with that. Note that Puppeteer now bundles its typing file, and
that some of the APIs have changed slightly, requiring a small adaptation
on our end.

[1]: https://developer.mozilla.org/en-US/docs/Web/API/Element/replaceChildren#browser_compatibility

Signed-off-by: Marc Dumais <[email protected]>
@marcdumais-work marcdumais-work merged commit 60209ec into master Feb 27, 2023
@marcdumais-work marcdumais-work deleted the update-puppeteer branch February 27, 2023 18:34
@github-actions github-actions bot added this to the 1.36.0 milestone Feb 27, 2023
Giangpro89 referenced this pull request Feb 28, 2023
The link to github was showing a 404.


Signed-off-by: Alexandra Buzila <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci issues related to CI / tests playwright issues related to playwright tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants