Remove deprecated actions and Playwright API usage

[planger]

What it does

• Replace usage of deprecated GabrielBB/xvfb-action@v1
• Replace usage of deprecated <obj>.type(...)

Contributed on behalf of STMicroelectronics.

How to test

Ensure workflows and Playwright tests still work as before.

Follow-ups

We should update https://github.com/eclipse-theia/theia-playwright-template and https://github.com/eclipse-theia/theia-e2e-test-suite accordingly to avoid using the deprecated GH action.

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[planger]

@xai Do you think omitting xvfb will be a problem for Electron? If yes, we'd need to switch to setup-xvfb as gabrielbb-xvfb-action is deprecated. Thanks!

[xai]

We need xvfb because we cannot run Electron headless. So yes, we should switch to the maintained action.

[planger]

Ok, no problem! I updated this PR. Can you please test if this works now? Thank you!

[xai]

I think we need to test by running the workflow, right?
Edit: I am of course testing the code change ;)

[xai]

Thank you very much, @planger, for spotting the unmaintained action we are consuming and for fixing our API calls!

Tested fine locally, i.e., the electron tests are still flaky, but no more flaky than before.

I agree that in the Electron case, we need to replace the unmaintained xvbf-action which had its last real commit in 2021-11.

The replacement used in this PR matches the recommendation in the discontinued xvbf-action and it seems to be actively maintained. It also seems to be used frequently (1/10 of the usage that the discontinued action has, which sounds reasonable to me). In the end, by executing the action, we end up executing this file: https://github.com/coactions/setup-xvfb/blob/main/dist/index.js

I very briefly scanned what it does and I have to mention that it includes odd-looking stuff such as this, which is explained with performance optimization. I did not spot any obvious red flags or outright harmful code, but keep in mind that this is a 4K lines javascript file and I looked at it for 3 minutes, which is not much, especially when considering that I am trying to limit my coffee input this week (read: I feel dumb and slow).

I assume that we need to establish a process for vetting third-party actions anyway at some point, and this workflow currently does not even seem to be executed by schedule or dispatch, so I will approve.

Edit: lol, my approval is, of course, not counting, because I am not a committer. If the actual reviewer finds the time, please also consider checking the 4K lines of javascript of the consumed action linked above.

[marcdumais-work]

The replacement used in this PR matches the recommendation in the discontinued xvbf-action and it seems to be actively maintained. It also seems to be used frequently (1/10 of the usage that the discontinued action has, which sounds reasonable to me). In the end, by executing the action, we end up executing this file: https://github.com/coactions/setup-xvfb/blob/main/dist/index.js

Good catch @xai - indeed it's hard to know for sure that this code is not doing anything we would not want it to. I looked at their issue tracker and noticed this issue, that claims that xvfb is now part of the Ubuntu 22.04 GitHub runner image, and that in consequence using the action is not longer needed, for that OS. It seems that we only use the action on Ubuntu, so maybe we can forego using that action and still be able to run Electron tests?

[marcdumais-work]

ATM this test suite does not seem to work - however it's the same on master, so it's not a problem caused by this PR.

I added trace "analyseTrace(): profilePath ..." - it seems that the trace files, that we attempt to parse using JSON.parse(), are binary files not json...

$ yarn -s electron rebuild && cd scripts/performance && node electron-performance.js --name 'Electron Frontend Startup' --folder electron --runs 10
native node modules are already rebuilt for electron
analyseTrace(): profilePath: /home/user/theia/scripts/performance/profiles/electron/1.json
[Performance][Electron Frontend Startup][1] Largest Contentful Paint (LCP) failed to obtain a measurement: Unexpected token o in JSON at position 1
Failed to obtain a measurement. The most likely cause is that the performance trace file was incomplete because the script did not wait long enough for "Largest Contentful Paint (LCP)".
SyntaxError: Unexpected token o in JSON at position 1
    at JSON.parse (<anonymous>)
    at analyzeTrace (/home/user/theia/scripts/performance/common-performance.js:151:26)
    at measure (/home/user/theia/scripts/performance/common-performance.js:86:26)
analyseTrace(): profilePath: /home/user/theia/scripts/performance/profiles/electron/2.json
[Performance][Electron Frontend Startup][2] Largest Contentful Paint (LCP) failed to obtain a measurement: Unexpected token o in JSON at position 1
Failed to obtain a measurement. The most likely cause is that the performance trace file was incomplete because the script did not wait long enough for "Largest Contentful Paint (LCP)".
SyntaxError: Unexpected token o in JSON at position 1
    at JSON.parse (<anonymous>)
    at analyzeTrace (/home/user/theia/scripts/performance/common-performance.js:151:26)
    at measure (/home/user/theia/scripts/performance/common-performance.js:86:26)

[...]

[planger]

Thank you very much for looking into this!
I get the same error (in this PR and on master). I think this workflow file has not been used for a long time, also I don't see any runs of this workflow on GH. I just updated it to avoid leaving the deprecated action there.

[xai]

I looked at their issue tracker and noticed this issue, that claims that xvfb is now part of the Ubuntu 22.04 GitHub runner image, and that in consequence using the action is not longer needed, for that OS. It seems that we only use the action on Ubuntu, so maybe we can forego using that action and still be able to run Electron tests?

Wow, nice! Thanks for checking this! This is really great news! If we have the xvfb-run binary in the image, we can save ourselves a lot of trouble.

[planger]

Thank you @xai and @marcdumais-work for your thorough review. I've updated the performance-tests.yml accordingly. However, as you also say, it doesn't work anyway (also on master) and I just updated it to get rid of the deprecated action and close the security risk, once someone picks up the work on this workflow again.

[marcdumais-work]

I just updated it to get rid of the deprecated action and close the security risk, once someone picks up the work on this workflow again.

I think that's fair. I was curious to confirm whether the image-built-in xvfb would work - I think we can trigger a manual run once the PR is merged, to confirm that it's at least present.

[marcdumais-work]

LGTM - thanks @planger !

[marcdumais-work]

I think we can trigger a manual run once the PR is merged, to confirm that it's at least present.

Confirmed, I think: we get the same parsing error I noticed locally, that we briefly discussed :

https://github.com/eclipse-theia/theia/actions/runs/8296738478/job/22706349277#step:7:20