Run playwright tests in CI

[planger]

What it does

• Add script yarn test:playwright for conveniently running playwright tests in the CI
• Include invocation of above-mentioned script in build.yml
  • Playwright tests are executed only once in ubuntu-18.04, Node.js v16.x

If you prefer, we could also run the playwright tests in a separate job on every PR / master commit.
Alternatively, we could only run the playwright tests periodically in a separate action.

I tend to prefer running the tests on every PR and master commit (within build.yml) either as part of the build job (as in this PR) or as a separate build job, as this raises breakages immediately when a change either broke expected behavior or broke the page objects. However, I understand that this would potentially add responsibilities to the committers. So alternatively, we could run those tests in a nightly job and we batch-fix the page objects periodically.

Please let me know what you think! Thanks!

How to test

• To only run playwright tests, run yarn && yarn build && yarn test:playwright
• Check CI build result of Build / ubuntu-18.04, Node.js v16.x

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]

@vince-fugnitto Thanks again for reviewing #10494!

I'd be very grateful if you could look at this PR too (as you already have the context) and let me know how whether you are fine with the way how the playwright build/verification is integrated.

Thanks!

[vince-fugnitto]

I tend to prefer running the tests on every PR and master commit (within build.yml) either as part of the build job (as in this PR) or as a separate build job, as this raises breakages immediately when a change either broke expected behavior or broke the page objects. However, I understand that this would potentially add responsibilities to the committers. So alternatively, we could run those tests in a nightly job and we batch-fix the page objects periodically.

@planger @JonasHelming should we make the decision in the next dev meeting? For example, who is expected to maintain these page object and how frequently do we update them (when they break vs before a release).

[JonasHelming]

@vince-fugnitto : Yes, let's at least point to this discussion in the meeting. I added it to the agenda.
My POV: I would also run them on every PR and also directly maintain them if they break. I expect maintenance to be very very minimal. Only if a core UI element is drastically changes, the mapping in the page object model has to be adapted, which in the current state almost never happens. If it happens, it will be much easier to fix it immediatly, because you know what you changed. In the time period this PR was open, ~70 other PRs were merged without any effect. If we experience an overhead with this, we can still change this process, i.e. extract it into a separate build and do a separate maintainace step. We are happy to observe this, support committers when something breaks and adapt the process if required.

[planger]

As discussed in the dev meeting, we decided to start with running the playwright tests on every PR & master commit and show the results in the list of checks, but don't make them blocking a merge if they happen to fail. This way we can observe their stability over a certain time period and decide later whether we make them a mandatory check or keep them just as information.

Thus, I moved the playwright test run into an own job in a separate workflow .github/workflows/playwright.yml, so the test run shows up as a separate check (see Playwright Tests / ubuntu-18.04, Node.js 16.x). Unfortunately, I can't see how this repo is configured regarding its branch rules due to missing permissions.
Are all checks currently configured to be mandatory? If yes, we will have to disable the Playwright Tests workflow in this list.

Also, I re-triggered the tests a couple of times to check their behavior on this infrastructure with this config and so far they look fine.

Please let me know if that meets the expectation as discussed in the dev meeting! Thanks a lot!

[planger]

Thanks a lot for your review @vince-fugnitto!

Hm, the 3PP License Check fail seems unrelated on a first glance, as we didn't any dependency in this change.
Or am I missing something?

WARN: Command [
  "java",
  "-jar",
  "/home/runner/work/theia/theia/scripts/download/dash-licenses.jar",
  "yarn.lock",
  "-batch",
  "50",
  "-timeout",
  "240",
  "-summary",
  "/home/runner/work/theia/theia/license-check-summary.txt"
] exited with code: 5
INFO: Checking results against the baseline...
X npm/npmjs/-/doctrine/2.1.0, Apache-2.0
ERROR: Found results that aren't part of the baseline!
error Command failed with exit code 1.

[vince-fugnitto]

@planger I believe there might have been an error on dash-licenses yesterday which resulted in a few deps being marked as false positives. The dependency is approved (twice actually):

• https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/1987
• https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/1981

I do have the pull-request #10725 which we can merge first if you'd like, it includes an upgrade and adds the approved deps to our baseline (until dash-licenses updates itself).

[planger]

@vince-fugnitto Ok great, thanks! Sure, let's merge PR #10725 first. I'm happy with the way you think is best. Thanks!

[vince-fugnitto]

@planger #10725 was merged so if you rebase the CI should be all green :)

[planger]

Thanks, rebased and keeping my fingers crossed :)

[vince-fugnitto]

LGTM 👍