testing: Add Theia Playwright framework (#10337)

[planger]

Fixes #10337

What it does

This change adds a page object framework based on Playwright that enables writing system tests (or end-to-end tests) of Theia-based applications more efficiently by introducing an abstraction layer over Theia's user interfaces, encapsulating the details of the user interface interactions, wait conditions, etc. This helps keeping your tests more concise, maintainable, and stable.

As part of this change, we also add tests for the introduced page objects, which run against the Theia example browser app in this repo. Currently, the framework is added as a separate package, which in a next step should be published and made consumable via npm, which we are happy to work on once this is merged.

How to test

With this change we added some initial documentation in the form of MD files on how to build, run and test the page object framework. Basically you'll have to run the following commands in the theia-playwright-testframework folder:

yarn
yarn theia:start &
yarn ui-tests

There are other scripts for running the tests headfully or only run specific tests (see documentation).

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

[vince-fugnitto]

I believe we should simplify the naming of the folder to simply playwright and add it under examples/ where we also have api-tests.

---

@JonasHelming I feel as though this may be a step backwards in the testing of the framework. We used to have tests that were written against the DOM/CSS and it was problematic. We decided to instead write integration tests at the API level (more info here).

Based on previous experience with writing DOM/CSS tests in the framework we saw that:

• tests were hard to write
• not robust
• subject to change more frequently
• had to rely on timing, slow
• and were overall difficult to maintain

I'm not sure about re-introducing DOM/CSS tests back to the framework. and instead we should think about adding more tests to the api-tests we have.

[JonasHelming]

I believe we should simplify the naming of the folder to simply playwright and add it under examples/ where we also have api-tests.

@JonasHelming I feel as though this may be a step backwards in the testing of the framework. We used to have tests that were written against the DOM/CSS and it was problematic. We decided to instead write integration tests at the API level (more info here).

Based on previous experience with writing DOM/CSS tests in the framework we saw that:

• tests were hard to write
• not robust
• subject to change more frequently
• had to rely on timing, slow
• and were overall difficult to maintain

I'm not sure about re-introducing DOM/CSS tests back to the framework. and instead we should think about adding more tests to the api-tests we have.

I think the danger with UI driven tests and system tests is to treat them as a replacement for unit tests or integration tests which they should not be. Philip outlines this in his talk at TheiaCon on slide 2 (https://www.youtube.com/watch?v=jDt9TU0Apts&t=24s)

The bottom line is that the coverage level should be very different for the different types of tests and system tests are not the type where you want to achieve a high coverage. The reason is exactly what you raise as concerns: higher maintenance and lower robustness. However, when developing a product you need a system test suite to ensure that what you actually deliver works as whole with a certain number of smoke tests. Otherwise you have to manually verify every build you deploy and there are errors that you will not detect.

So i agree with you that you should never replace unit tests or integration tests with system tests, but I also believe you cannot replace systems tests with unit or integration tests.

This PR does not suggest a change of the test strategy, but it provides a helper framework to augment it with system tests. This helper framework btw addresses very exactly the shortcomings your raised by abstracting the direct dependency to the DOM elements. Having a common solution for this problem is very essential for adopting projects from my POV

[vince-fugnitto]

@JonasHelming

However, when developing a product you need a system test suite to ensure that what you actually deliver works as whole with a certain number of smoke tests.

My concern is I'm not sure it is up to the framework to define which testing framework downstream application should use, and which smoke tests should be run. Is it not up to actual downstream products to choose their testing framework and smoke tests they are interested in? We already have the API integration tests in the framework to test end-to-end without the flakiness of the browser. Is this testing framework more suitable to blueprint which is a product, rather than the framework directly?

[JonasHelming]

I think we have to clearly separate the testing framework and the actual smoke tests.

For the framework, I see much value in providing this as a project. The page object model is the same and if e do not provide it in a central way, every adopter has to re implement it. This is what we could already observe and this was the motivation to contribute it so we allow to reuse it and share the maintenance. It i true that we pre choose the testing framework here. It is not an arbitrary choice, we have use Cypress and others before and ended up with Playwright as the best choice. Adopters can still use another framework. However, I would claim that if you can chose between a framework with a page object model available or something else, I would usually go for the first.

About the actual smoke tests: Yes, I agree these would also fit into Blueprint. However, I believe this PR only contains a very small number of tests which smoke test some core UI components. It might make sense to have some very basic tests in here to smoke test the framework and to continuously validate the page object model itself. My proposal would be that if they turn out to be flaky, we can still move them somewhere else.

[planger]

Thanks for the feedback @vince-fugnitto and @JonasHelming!

The tests that are included in the page object library of this PR aim at testing the page objects in this library against the latest master of Theia. They are not intended to test Theia itself. If a test fails, it is expected that the respective page object needs to be adjusted in order to be compatible with the latest Theia --- not the other way around. This way, we can ensure that we would ship a stable page object library for a specific version of Theia alongside the Theia releases.

Adopters can then upgrade the page object library dependency when they upgrade their consumed Theia version and consequently would likely not have to change their specific system tests for their Theia-based application. Thus, the effort of maintaining their system tests, including potential wait conditions for getting the timing right, etc., is kept to a minimum and effort of maintaining the page object library itself is shared with the community.

This is all aiming exactly at mitigating the issues with system tests that run against the DOM that have been mentioned above.

I'm happy to apply the changes you requested @vince-fugnitto in the next one or two days.

[planger]

Thanks again @vince-fugnitto for the feedback! I appended two commits which address your review comments.

[colin-grant-work]

I think there are (at least) three separate questions here:

1. Is it worth making and open-sourcing such a testing framework?
   • I think the answer is a definite yes: there's known demand for it, and making one version publicly available will make it easier for adopters who want such a thing to get it and use it.
2. Do we want to use it for (PR-gating) tests in this repository?
   • Here, the answer is less clear. As Vince has pointed out, there used to be DOM-oriented tests, and that approach was eventually rejected. I don't believe that the framework in use at the time was as sophisticated as this, but the fact remains that these tests are slower and likely to be less robust than the current testing setup. @planger's comment suggests that including this code here is mainly aimed at testing this code's page object model, rather than testing Theia. To achieve that end, the code doesn't have to be here - it could be somewhere else and run a nightly job against this repo's master branch, for example.
3. Closely related to (2), does this code belong here, in Blueprint, or somewhere else (stand-alone repo under the organization, ...)?
   • There are a number of things to consider here:
     • whether and how we want to make use of the code in this repo
     • who should / wants to be responsible for maintaining the code. If it's in this repo, then it becomes the task of all committers to the repository to make sure it's working - we specialize, but ultimately, we're all on the hook for making the whole thing work.
     • how we plan to structure the larger Theia ecosystem, including derived Eclipse products, extensions maintained within the Theia organization, third-party extensions, etc.

There are enough factors to weigh there that I won't venture to make an argument one way or the other, but it deserves discussion.

[JonasHelming]

@vince-fugnitto @colin-grant-work @marcdumais-work : Based on the discussion we had in the dev meeting, my conclusion was that we are good to merge this. Any objections?

[vince-fugnitto]

…my conclusion was that we are good to merge this. Any objections?

@JonasHelming we should probably at least review it first (after being rebased) :) I believe we can do so in the new year since the majority of the committers might be on holiday vacations.

[JonasHelming]

@vince-fugnitto : Sure, I thought you already did a review and I was not expecting anybody to answer this week :-)
Happy holidays!!

[vince-fugnitto]

@planger do you mind updating the pull-request to resolve the build issues, it looks like there are uncommitted changes to the yarn.lock file which fails all the builds. In addition, please be sure to squash as there should not be any merge commits present in any pull-request (fe0931c).

I'll perform a thorough review afterwards.

[planger]

@vince-fugnitto Thank you very much! I rebased the changes and squashed all commits. The build passes however the license check fails because of new dependencies. Is there anything I should change to fix that?

Thanks again!

[vince-fugnitto]

@vince-fugnitto Thank you very much! I rebased the changes and squashed all commits. The build passes however the license check fails because of new dependencies. Is there anything I should change to fix that?

@planger if we can, can we not update dependencies (ex: eslint-plugin-deprecation), as it might require an additional CQ as this depedency was exempted for us. It seems that a yarn upgrade was maybe performed?

In addition, the bump of eslint-plugin-deprecation is problematic as it causes the following issue: eclipse-theia/theia-blueprint#191 (review).

For other issues we will need to do the process of submitting reports for the use of them (which in turn updates dash-licenses) @marcdumais-work can help with that when he is back from vacations.

[planger]

@planger if we can, can we not update dependencies (ex: eslint-plugin-deprecation), as it might require an additional CQ as this depedency was exempted for us. It seems that a yarn upgrade was maybe performed?

Oh sure, sorry! I reset all changes to the yarn.lock and re-ran the build. So the latest commit should contain the minimal changes. It looks much better now and, on a first sight, doesn't seem to actually change any dependencies. It only adds a few and maps new versions to existing dependencies.

In addition, the bump of eslint-plugin-deprecation is problematic as it causes the following issue: eclipse-theia/theia-blueprint#191 (review).

I see! The upgrade of eslint-plugin-deprecation should be removed now.

For other issues we will need to do the process of submitting reports for the use of them (which in turn updates dash-licenses) @marcdumais-work can help with that when he is back from vacations.

Thanks a lot!

[planger]

Hi @vince-fugnitto, thanks again for your support so far!

Do you have time to take another look at this PR? The builds (except for the license check, which is expected) are green. If you want me to rebase again, please let me know. Thanks a lot!

[vince-fugnitto]

@planger I have a few comments about the code, from what I ran it seemed to be passing 👍

I opened the following IP Due Diligence issues for the added dependencies which were failing the CI check using the automated tool (all approved):

• https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/1924
• https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/1925
• https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/1926
• https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/1927

I remember a mention that the tests should pass and be released with a given theia release but nothing at the moment runs the tests, I'm not sure how feasible it will be to remember to maintain the tests if not automated. Should we not have a CI job to confirm the tests pass (but would not fail the build)?

[planger]

@vince-fugnitto Thank you very much for your review and for taking care of the IP due diligence!

I've force pushed a rebased and squashed commit on top of current master (there was a conflict in yarn.lock). With this commit, all your suggestions are incorporated. I've also applied the readme template to the @theia/playwright readme and removed the compilation target to fall back to Theia's default target (ES2017).

I remember a mention that the tests should pass and be released with a given theia release but nothing at the moment runs the tests, I'm not sure how feasible it will be to remember to maintain the tests if not automated. Should we not have a CI job to confirm the tests pass (but would not fail the build)?

Right, we'll set up the build jobs for publishing @theia/playwright and running the tests right after this is merged. Is that fine with you? I think it is easier to do that, after this change is already in.

Thanks again!

[vince-fugnitto]

The changes look good to me 👍

I confirmed that the added tests pass successfully:

Test Output

~/workspaces/theia/examples/playwright $ yarn ui-tests
yarn run v1.22.4
$ yarn && playwright test --config=./configs/playwright.config.ts
[1/5] Validating package.json...
[2/5] Resolving packages...
success Already up-to-date.
$ yarn clean && yarn build
$ rimraf lib
$ tsc --incremental && yarn lint && npx playwright install chromium
$ eslint -c ./.eslintrc.js --ext .ts ./src ./tests
Using config at /home/evinfug/workspaces/theia/examples/playwright/configs/playwright.config.ts

Running 48 tests using 2 workers

  ✓  ../../tests/theia-app.test.ts:26:9 › Theia Application › should load (6s)
  ✓  ../../tests/theia-app.test.ts:30:9 › Theia Application › should show main content panel (67ms)
  ✓  ../../tests/theia-explorer-view.test.ts:34:9 › Theia Explorer View › should be visible and acti
  ✓  ../../tests/theia-explorer-view.test.ts:40:9 › Theia Explorer View › should be opened at the le
  ✓  ../../tests/theia-explorer-view.test.ts:46:9 › Theia Explorer View › should be possible to clos
  ✓  ../../tests/theia-main-menu.test.ts:31:9 › Theia Main Menu › should show the main menu bar (85m
  ✓  ../../tests/theia-main-menu.test.ts:38:9 › Theia Main Menu › should open main menu 'File' (122m
  ✓  ../../tests/theia-main-menu.test.ts:43:9 › Theia Main Menu › should show the menu items 'New Fi
  ✓  ../../tests/theia-main-menu.test.ts:50:9 › Theia Main Menu › should return menu item by name 'N
  ✓  ../../tests/theia-main-menu.test.ts:65:9 › Theia Main Menu › should detect whether menu item ha
  ✓  ../../tests/theia-main-menu.test.ts:74:9 › Theia Main Menu › should be able to show menu item i
  ✓  ../../tests/theia-main-menu.test.ts:82:9 › Theia Main Menu › should close main menu (82ms)
  ✓  ../../tests/theia-preference-view.test.ts:30:9 › Preference View › should be visible and active
  ✓  ../../tests/theia-explorer-view.test.ts:56:9 › Theia Explorer View › should show one folder nam
  ✓  ../../tests/theia-explorer-view.test.ts:79:9 › Theia Explorer View › should provide file stat n
  ✓  ../../tests/theia-explorer-view.test.ts:86:9 › Theia Explorer View › should provide file stat n
  ✓  ../../tests/theia-explorer-view.test.ts:97:9 › Theia Explorer View › should provide file stat n
  ✓  ../../tests/theia-explorer-view.test.ts:103:9 › Theia Explorer View › should open context menu 
  ✓  ../../tests/theia-explorer-view.test.ts:117:9 › Theia Explorer View › should rename "sample.txt
  ✓  ../../tests/theia-preference-view.test.ts:37:9 › Preference View › should be able to read, set,
  ✓  ../../tests/theia-preference-view.test.ts:52:9 › Preference View › should be able to read, set,
  ✓  ../../tests/theia-preference-view.test.ts:67:9 › Preference View › should throw an error if we 
  ✓  ../../tests/theia-problems-view.test.ts:30:9 › Theia Problems View › should be visible and acti
  ✓  ../../tests/theia-preference-view.test.ts:84:9 › Preference View › should throw an error if we 
  ✓  ../../tests/theia-quick-command.test.ts:34:9 › Theia Quick Command › should show quick command 
  ✓  ../../tests/theia-quick-command.test.ts:39:9 › Theia Quick Command › should trigger 'About' com
  ✓  ../../tests/theia-problems-view.test.ts:37:9 › Theia Problems View › should be opened at the bo
  ✓  ../../tests/theia-problems-view.test.ts:44:9 › Theia Problems View › should be closable (246ms)
  ✓  ../../tests/theia-problems-view.test.ts:54:9 › Theia Problems View › should not throw an error 
  ✓  ../../tests/theia-status-bar.test.ts:34:9 › Theia Status Bar › should show status bar (15ms)
  ✓  ../../tests/theia-status-bar.test.ts:38:9 › Theia Status Bar › should contain status bar elemen
  ✓  ../../tests/theia-text-editor.test.ts:32:9 › Theia Text Editor › should be visible and active a
  ✓  ../../tests/theia-text-editor.test.ts:39:9 › Theia Text Editor › should be possible to open "sa
  ✓  ../../tests/theia-text-editor.test.ts:47:9 › Theia Text Editor › should be possible to open fou
  ✓  ../../tests/theia-quick-command.test.ts:53:9 › Theia Quick Command › should trigger 'Toggle Exp
  ✓  ../../tests/theia-text-editor.test.ts:79:9 › Theia Text Editor › should return the contents of 
  ✓  ../../tests/theia-text-editor.test.ts:87:9 › Theia Text Editor › should return the contents of 
  ✓  ../../tests/theia-text-editor.test.ts:95:9 › Theia Text Editor › should be dirty after changing
  ✓  ../../tests/theia-text-editor.test.ts:105:9 › Theia Text Editor › should replace the line with 
  ✓  ../../tests/theia-text-editor.test.ts:116:9 › Theia Text Editor › should replace the line with 
  ✓  ../../tests/theia-text-editor.test.ts:123:9 › Theia Text Editor › should delete the line with c
  ✓  ../../tests/theia-text-editor.test.ts:130:9 › Theia Text Editor › should delete the line with l
  ✓  ../../tests/theia-text-editor.test.ts:137:9 › Theia Text Editor › should have more lines after 
  ✓  ../../tests/theia-text-editor.test.ts:150:9 › Theia Text Editor › should undo and redo text cha
  ✓  ../../tests/theia-workspace.test.ts:25:9 › Theia Workspace › should be initialized empty by def
  ✓  ../../tests/theia-text-editor.test.ts:167:9 › Theia Text Editor › should close without saving (
  ✓  ../../tests/theia-workspace.test.ts:32:9 › Theia Workspace › should be initialized with the con
  ✓  ../../tests/theia-workspace.test.ts:41:9 › Theia Workspace › should be initialized with the con

  Slow test file: ../../tests/theia-quick-command.test.ts (29s)
  Slow test file: ../../tests/theia-text-editor.test.ts (20s)
  Slow test file: ../../tests/theia-explorer-view.test.ts (16s)
  Consider splitting slow test files to speed up parallel execution

  48 passed (1m)
Done in 89.48s.

Thank you for your patience and responsiveness throughout the review :)

[planger]

Awesome! 🥳 Thank you very much!
We'll now start working on the build and publishing jobs.

[JonasHelming]

More details about this feature see here

[christian-bromann]

Hey all 👋
just wanted to let you know that I've been working on something similar for VSCode which is based on Webdriver.io which you can find here. The set of page objects I've created should be easily adaptable for Theia given applications are fairly similar and the approach I took works with locatorMaps that allows to pick a specific map for an IDE version or flavor. I hope a vendor neutral alternative such as WebdriverIO can be at some point in the future an option for Theia developers. Playwright, which is controlled by Microsoft and their automation is not based on web standards, might be for many folks a no-go.

[planger]

@christian-bromann This is interesting, thanks for the pointer! It is always great to have multiple options and I'd be certainly interested to give it a try once it works with Theia.