ci: fix browser-tests ubuntu 24.04 compat by legobeat · Pull Request #2587 · endojs/endo

legobeat · 2024-10-16T21:24:18Z

Fix: #2660

Description

Fix recent failing CI browser tests

ci: use npm-run instead of npx to run playwright
ci: manually install playwright system dependencies

Context

Security Considerations

N/A

Scaling Considerations

N/A

Documentation Considerations

Give our docs folks some hints about what needs to be described to downstream users. Backwards compatibility: what happens to existing data or deployments when this code is shipped? Do we need to instruct users to do something to upgrade their saved data? If there is no upgrade path possible, how bad will that be for users?

Testing Considerations

Every PR should of course come with tests of its own functionality. What additional tests are still needed beyond those unit tests? How does this affect CI, other test automation, or the testnet?

Compatibility Considerations

N/A

Upgrade Considerations

N/A

leotm · 2024-10-17T07:09:26Z

+      - name: Install browser dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends \
+            ca-certificates \
+            libatomic1 libflite1 libgles2 libharfbuzz-icu0 libhyphen0 libmanette-0.2-0 libicu74 libsecret-1-0 libxslt1.1 woff2 \
+            ca-certificates fonts-liberation libatk-bridge2.0-0 libatk1.0-0 libc6 libcairo2 libcups2 libdbus-1-3 libexpat1 libfontconfig1 libgbm1 libgcc1 libglib2.0-0 libgtk-3-0 libnspr4 libnss3 libpango-1.0-0 libpangocairo-1.0-0 libstdc++6 libx11-6 libx11-xcb1 libxcb1 libxcomposite1 libxcursor1 libxdamage1 libxext6 libxfixes3 libxi6 libxrandr2 libxrender1 libxss1 libxtst6 lsb-release wget xdg-utils \
+            libvpx9 libevent-2.1-7t64 libopus0 libgstreamer-plugins-base1.0-0 libgstreamer-gl1.0-0 libgstreamer-plugins-bad1.0-0 gstreamer1.0-libav


are all of these extra deps required? or can any be left out to be more minimal

They may not all be required. Most if not all were previously installed under the hood by the playwright util.

We should definitely install only the minimum needed

This seems like a future maintenance burden

Linebreaks seem inconsistent

Are you sure changes needed to the action runner? This looks like a Playwright issue that was fixed in 1.45.

boneskull

I don't understand the motivation behind this change. Can you update the PR description?

boneskull · 2024-10-25T20:21:01Z

+      - name: Install browser dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends \
+            ca-certificates \
+            libatomic1 libflite1 libgles2 libharfbuzz-icu0 libhyphen0 libmanette-0.2-0 libicu74 libsecret-1-0 libxslt1.1 woff2 \
+            ca-certificates fonts-liberation libatk-bridge2.0-0 libatk1.0-0 libc6 libcairo2 libcups2 libdbus-1-3 libexpat1 libfontconfig1 libgbm1 libgcc1 libglib2.0-0 libgtk-3-0 libnspr4 libnss3 libpango-1.0-0 libpangocairo-1.0-0 libstdc++6 libx11-6 libx11-xcb1 libxcb1 libxcomposite1 libxcursor1 libxdamage1 libxext6 libxfixes3 libxi6 libxrandr2 libxrender1 libxss1 libxtst6 lsb-release wget xdg-utils \
+            libvpx9 libevent-2.1-7t64 libopus0 libgstreamer-plugins-base1.0-0 libgstreamer-gl1.0-0 libgstreamer-plugins-bad1.0-0 gstreamer1.0-libav


We should definitely install only the minimum needed

This seems like a future maintenance burden

Linebreaks seem inconsistent

boneskull · 2024-10-25T20:21:13Z

      - name: Install Playwright Browsers
        working-directory: browser-test
-        run: npx playwright install --with-deps
+        run: npm exec playwright install --with-deps


Doesn't npx do the same thing if playwright is local?

atm should be

Suggested change

run: npm exec playwright install --with-deps

run: npm exec -- playwright install --with-deps

Doesn't npx do the same thing if playwright is local?

yep npx is nice shortcut here too

let's stick with npx since

less to write

don't have to remember --

official docs (if cli counts)

agreed, please leave that

leotm · 2024-10-29T19:40:21Z

I don't understand the motivation behind this change. Can you update the PR description?

updated ^ it's to fix the recent failing Browser Tests happening in a few PRs

boneskull · 2024-11-25T20:34:43Z

Is this still a problem?

turadg

Thanks for tackling this! I think the solution may be a simple version bump to get the fix in 1.45.

turadg · 2024-12-13T16:43:59Z

    timeout-minutes: 30
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-24.04
+    # runs-on: ubuntu-latest


isn't latest 24.04? did ubuntu-latest cause a problem or are you just wanting to defend against that changing?

I think we should stick with ubuntu-latest because that's what Playwright advises with GitHub Actions:
https://playwright.dev/docs/ci-intro#setting-up-github-actions

It is, now, but it was unpredictably (A/B from GH side?) switching back and forth between 22.04 and 24.04 between CI runs for a period around at the time of PR creation, which was causing inconsistent CI results.

Not clear if the next transition of ubuntu-latest will be cleaner or similarly messy and require a similar workaround whenever it happens.

turadg · 2024-12-13T16:48:18Z

      - name: Install Playwright Browsers
        working-directory: browser-test
-        run: npx playwright install --with-deps
+        run: npm exec playwright install --with-deps


agreed, please leave that

turadg · 2024-12-13T16:53:39Z

+      - name: Install browser dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends \
+            ca-certificates \
+            libatomic1 libflite1 libgles2 libharfbuzz-icu0 libhyphen0 libmanette-0.2-0 libicu74 libsecret-1-0 libxslt1.1 woff2 \
+            ca-certificates fonts-liberation libatk-bridge2.0-0 libatk1.0-0 libc6 libcairo2 libcups2 libdbus-1-3 libexpat1 libfontconfig1 libgbm1 libgcc1 libglib2.0-0 libgtk-3-0 libnspr4 libnss3 libpango-1.0-0 libpangocairo-1.0-0 libstdc++6 libx11-6 libx11-xcb1 libxcb1 libxcomposite1 libxcursor1 libxdamage1 libxext6 libxfixes3 libxi6 libxrandr2 libxrender1 libxss1 libxtst6 lsb-release wget xdg-utils \
+            libvpx9 libevent-2.1-7t64 libopus0 libgstreamer-plugins-base1.0-0 libgstreamer-gl1.0-0 libgstreamer-plugins-bad1.0-0 gstreamer1.0-libav


Are you sure changes needed to the action runner? This looks like a Playwright issue that was fixed in 1.45.

leotm · 2025-01-14T12:27:38Z

superseded currently by #2686 i believe (the bump mentioned in #2587 (comment))

naugtur · 2025-01-14T13:15:17Z

superseded currently by #2686 i believe (the bump mentioned in #2587 (comment))

Yes, that was the minimal update needed to get it to work again.
We could consider pinning ubuntu version in case github starts A/B testing releases again - 24.04 is LTS so we're better off on it than updating to 25 whenever it becomes latest.

legobeat force-pushed the ci-fix-browser-test branch 7 times, most recently from 8e536e0 to a710787 Compare October 16, 2024 23:42

legobeat marked this pull request as ready for review October 16, 2024 23:42

legobeat mentioned this pull request Oct 16, 2024

fix(ses): completePrototypes on Hermes #2563

Closed

leotm reviewed Oct 17, 2024

View reviewed changes

leotm approved these changes Oct 17, 2024

View reviewed changes

legobeat force-pushed the ci-fix-browser-test branch from a710787 to e9f230e Compare October 18, 2024 03:31

legobeat added 4 commits October 24, 2024 01:00

ci: use npm-exec instead of npx to run playwright

a16add6

devDeps(test): @playwright/test@^1.48.1

006721a

ci(browser-test): runs-on ubuntu-24.04

deed708

ci: manually install playwright system dependencies

c0acd95

legobeat force-pushed the ci-fix-browser-test branch from e9f230e to c0acd95 Compare October 24, 2024 01:00

boneskull reviewed Oct 25, 2024

View reviewed changes

leotm requested a review from turadg December 13, 2024 12:47

leotm mentioned this pull request Dec 13, 2024

Flakey browser tests (playwright) #2660

Closed

turadg requested changes Dec 13, 2024

View reviewed changes

leotm mentioned this pull request Jan 14, 2025

chore: Fix browser tests #2686

Merged

naugtur closed this Jan 14, 2025

	run: npm exec playwright install --with-deps
	run: npm exec -- playwright install --with-deps

Conversation

legobeat commented Oct 16, 2024 • edited by leotm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Compatibility Considerations

Upgrade Considerations

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boneskull left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leotm commented Oct 29, 2024

Uh oh!

boneskull commented Nov 25, 2024

Uh oh!

turadg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leotm commented Jan 14, 2025

Uh oh!

naugtur commented Jan 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

legobeat commented Oct 16, 2024 •

edited by leotm

Loading

boneskull left a comment •

edited

Loading