-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RHOAIENG-15152: feat(codeserver/e2e): add initial playwright e2e test project skeleton #755
RHOAIENG-15152: feat(codeserver/e2e): add initial playwright e2e test project skeleton #755
Conversation
The CI fails are missing |
One funny realization; depending on the platform where you run codeserver (or most likely where and what browser you're opening it with), the buttons for trusting and not trusting workspace are switched. Either the ack button is on the left, or on the right. Never realized that it actually changed order for me after I switched from KDE Plasma to MacOS! |
@@ -134,7 +134,7 @@ jobs: | |||
if: "${{ fromJson(inputs.github).event_name == 'pull_request' }}" | |||
env: | |||
IMAGE_TAG: "${{ github.sha }}" | |||
IMAGE_REGISTRY: "localhost:5000/workbench-images" | |||
IMAGE_REGISTRY: "ghcr.io/${{ github.repository }}/workbench-images" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We aren't pushing the image any more as we had to in the beginning, so we can use any registry we like; so let's keep it consistent.
* check env variables to determine how to run * take screenshots on failure * don't open annoying html report * disable parallel runs
dcbd1ea
to
42f0d28
Compare
I didn't actually run this on my machine. LGTM in general. Do we want to create this stub only in the codeserver indeed? If we're introducing this, don't we want to create it somehow generic, so we can reuse at least portions of code/tests between the diferent images? Also, looks like the Notebooks 2.0 will use Cypress in the end: kubeflow/notebooks#75. Should we be concerned? 🤔 |
Here's nothing to run, actually. It's not a walking skeleton, just a skeleton. I originally intended to keep this in code server for the time being, but you're right that maybe starting in tests/e2e or some directory like that makes more sense. I'll do that change tomorrow.
Regarding cypress, I am actually not concerned. Playwright is a better design (real browser automation library) more suitable for what we're doing. if we ever wanted to copy code from somebody else's cypress tests, we can do that with not too much effort. |
42f0d28
to
630eec6
Compare
@jiridanek: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
/override ci/prow/images |
@jiridanek: Overrode contexts on behalf of jiridanek: ci/prow/images In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jiridanek The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
… project skeleton (opendatahub-io#755) * NO-ISSUE: feat(codeserver/e2e): add initial playwright project skeleton * NO-ISSUE: feat(codeserver/e2e): add a basic README.md * NO-ISSUE: add testcontainers as a dependency * NO-ISSUE: new customizable and flexible config * check env variables to determine how to run * take screenshots on failure * don't open annoying html report * disable parallel runs * exclude pnpm-lock.yaml from yamllint * move Playwright tests from codeserver/e2e to tests/browser * create a new containers folder for (future) testing of containers in Python (cherry picked from commit 768ae5c)
… project skeleton (opendatahub-io#755) * NO-ISSUE: feat(codeserver/e2e): add initial playwright project skeleton * NO-ISSUE: feat(codeserver/e2e): add a basic README.md * NO-ISSUE: add testcontainers as a dependency * NO-ISSUE: new customizable and flexible config * check env variables to determine how to run * take screenshots on failure * don't open annoying html report * disable parallel runs * exclude pnpm-lock.yaml from yamllint * move Playwright tests from codeserver/e2e to tests/browser * create a new containers folder for (future) testing of containers in Python (cherry picked from commit 768ae5c)
Made for https://issues.redhat.com/browse/RHOAIENG-14518 but it's essentially unrelated work.
Filled new Jira just for the test https://issues.redhat.com/browse/RHOAIENG-15152
Description
As you are well aware, we've been struggling to pick our e2e toolset and to write some left-shifted tests. Thankfully, we have a helpful Architect in Red Hat Middleware-Application Services (hello David), who recommended Playwright in TypeScript as the only sensible e2e option for web stuff that even developers are willing to touch.
I did consider Cypress and various Selenium bindings; I would not mind switching what I have here into that if it's more liked in the team. Just I think we should polish and merge this Playwright first, even if we're going to abandon it, so that we at least have something.
The
await
porn is even more stupid than I expected that to be. Mandatory reading https://journal.stuffwithstuff.com/2015/02/01/what-color-is-your-function/ And there's some theory about effect systems if you want to discuss this with academically minded people!?!I'm using the Playwright test runner. That's by no means required; it's possible to use just the automation library part of Playwright and structure the tests differently. I guess I decided that since I don't have any test runner I like in the JavaScript world, I may just as well pick the Playwright one. It has fixtures like Pytest, so there may be some familiarity there.
Besides that, we have here testcontainers, because they are also the most popular and therefore should be relatively unobjectionable choice for a library to start a container in the background.
What does it look like
Playwright has a fancy html report. I do pretty much agree with @zhimin's prolific blogs that are saying that all that is not actually well suited for a traditional QE team (edit: that's working in true agile way), but we aren't that, and also the number of e2e tests we're going to have here to run on every PR is going to be small. We're still free to test differently in QE (or DevTestOps as we now say).
What's it like to work with
The tests can be set to run against fresh browser as well as a browser you provide (the
--remote-debugging-port
is what does the trick). Check outplaywright.config.ts
where you can switch between the two modes.In the same vein, the tests can run against already started codeserver image, or they can start its own container, fresh one for every test. (I'm not sure if we should try to reuse codeserver instance or start fresh every time. Fresh should make writing tests easier as cleanup becomes unnecessary and the cost in increased test runtime is I think worth that. If we add more suites besides this "basic functionality" one, then maybe we can reassess that there. Good test hygiene is hard, especially keeping it up for a longer time.)
Debugging tests
Running against provided browser and provided codeserver, possibly commenting out few unnecessary steps before the interesting part, it is usually possible to iterate on the thing quickly and get it working. Then check with fresh browser and fresh image.
Playwright has the
codegen
andtest --ui
tools. The codegen is ok to give you decent locators to use, and the ui is not bad either; video recording on steroids.The
test --ui
thing is pretty much what Cypress has also; the thing we were so impressed with in Savitha's demo earlier this year.For some reason, default Playwright settings will popup a new browser tab with report every time a test run fails. I've disabled that popup thing and instead enabled the
line
reporter that gives you some stacktrace; thats way more reasonable.I do have two fancy test helpers in some utils file. They are sufficiently complex that there may need to be special tests to test these tests (helpers) ;P
The tests themselves
I essentially copied that from Coder Inc. I did preserve copyright header and put MIT licence, same as they have, so we should be good. The difference from them is that they control
code-server
invocation, whereas for us the parameters given to it are fixed. So my tests can be simpler.How Has This Been Tested?
GHA and played with it on my machine.
Merge criteria:
I'll cleanup the git history later. Please have a look now nevertheless.