Skip to content

Conversation

@ElectricalBoy
Copy link
Collaborator

@ElectricalBoy ElectricalBoy commented Aug 27, 2025

@ElectricalBoy ElectricalBoy marked this pull request as ready for review August 27, 2025 06:04
@Rathoz
Copy link
Collaborator

Rathoz commented Aug 27, 2025

Not sure I understand why this approach?

Why not just do the same as https://playwright.dev/docs/ci#docker, but with npm run lua-test instead of npx playwright test?

@ElectricalBoy
Copy link
Collaborator Author

ElectricalBoy commented Aug 27, 2025

Not sure I understand why this approach?

Why not just do the same as https://playwright.dev/docs/ci#docker, but with npm run lua-test instead of npx playwright test?

because mcr.microsoft.com/playwright:v1.55.0-noble does not include sudo, using our current lua setup action, and thus, running the entire workflow inside the container, is a no-go (unless we install sudo inside container before doing Lua stuff)

https://github.com/lewis6991/gh-actions-lua/blob/01aab24c4de9555717b685f9b142a2abbe12ef14/main.js#L223

@Rathoz
Copy link
Collaborator

Rathoz commented Aug 27, 2025

Try going back to your oroginal attempt and do the following changes:

options: --user root in the container setup

      - name: Create a fake sudo wrapper
        run: |
          echo '#!/bin/sh' > /usr/local/bin/sudo
          echo 'exec "$@"' >> /usr/local/bin/sudo
          chmod +x /usr/local/bin/sudo

@Rathoz Rathoz changed the title ci: run visual snapshot workflow on container ci: improve visual snapshot workflow setup time Aug 27, 2025
@ElectricalBoy ElectricalBoy force-pushed the speed-up-snapshot branch 2 times, most recently from a69a9e2 to baafe8b Compare August 27, 2025 12:21
@ElectricalBoy
Copy link
Collaborator Author

Try going back to your oroginal attempt and do the following changes:

options: --user root in the container setup

      - name: Create a fake sudo wrapper
        run: |
          echo '#!/bin/sh' > /usr/local/bin/sudo
          echo 'exec "$@"' >> /usr/local/bin/sudo
          chmod +x /usr/local/bin/sudo

doesn't work either; 8df1fb5...a69a9e2

  • all the build dependencies have to be installed from scratch (GH's runner images already have them loaded)
  • apt-get does not have package list loaded (most containers don't anyway), so container needs to load them fresh
  • the above two add enough complexity making this approach not preferable

@Rathoz
Copy link
Collaborator

Rathoz commented Aug 27, 2025

If we fork gh-lua-acitons (it's dead anyway), if we remove the sudo, would it work?

@ElectricalBoy
Copy link
Collaborator Author

ElectricalBoy commented Aug 27, 2025

If we fork gh-lua-acitons (it's dead anyway), if we remove the sudo, would it work?

technically yeah, but at that point I think we should be prebuilding our own container image with lua installed instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants