-
Notifications
You must be signed in to change notification settings - Fork 383
[RI-6570] Playwright E2E tests #4615
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
base: main
Are you sure you want to change the base?
Conversation
plus comments
Code Coverage - Integration Tests
|
Code Coverage - Backend unit tests
Test suite run success2941 tests passing in 286 suites. Report generated by 🧪jest coverage report action from c01b123 |
Code Coverage - Frontend unit tests
Test suite run success4792 tests passing in 628 suites. Report generated by 🧪jest coverage report action from c01b123 |
|
The electron tests fail with the following on Linux (after updating the path to the built version. Do they work on Mac? Running 7 tests using 1 worker (Use (Use
[Chromium] › tests/keys.spec.ts:48:9 › Adding Database Keys › Verify that user can add Set Key (Use
[Chromium] › tests/keys.spec.ts:58:9 › Adding Database Keys › Verify that user can add List Key (Use
[Chromium] › tests/keys.spec.ts:68:9 › Adding Database Keys › Verify that user can add String Key (Use
[Chromium] › tests/keys.spec.ts:78:9 › Adding Database Keys › Verify that user can add ZSet Key (Use
[Chromium] › tests/keys.spec.ts:89:9 › Adding Database Keys › Verify that user can add Stream key (Use
6 failed Serving HTML report at http://localhost:37641. Press Ctrl+C to quit. |
@@ -6,3 +6,4 @@ RI_NOTIFICATION_UPDATE_URL=https://s3.amazonaws.com/redisinsight.test/public/tes | |||
RI_NOTIFICATION_SYNC_INTERVAL=30000 | |||
RI_FEATURES_CONFIG_URL=http://static-server:5551/remote/features-config.json | |||
RI_FEATURES_CONFIG_SYNC_INTERVAL=50000 | |||
TEST_BIG_DB_DUMP=https://s3.amazonaws.com/redisinsight.test/public/rte/dump/big/dump.tar.gz |
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.
It's a bit odd that we have an .env
file committed in the repo, but yeah - this file was here before your PR.
Still, I believe it's better to keep these configs as environment variables - maybe we can offload it as a separate tech debt and take a look later?
@@ -0,0 +1,130 @@ | |||
/* eslint-disable max-len */ |
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.
Why is this necessary, or is it a leftover from some tests?
"generateAndShowReports": "allure serve allure-results", | ||
"test:autogen": "playwright codegen" | ||
}, | ||
"dependencies": { |
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.
I see playwright
itself is not part of the dependencies list, is that intentional?
I'm asking because I had issues going through the README instructions. I had to manually add it as a global dependency in order to run the following commands:
yarn playwright install
sudo yarn playwright install-deps
So maybe we should add playwright
to this list?
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.
I think playwright along with it's browsers need to be installed globally
in their setup guide and test project "playwright" is not included as dependency either
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.
I looked through the official Playwright docs and a few example projects provided by Microsoft, and I think we should at least include @playwright/test
as a dev dependency. Installing it globally without listing it in package.json
can lead to version mismatches between CI and local dev environments, which could cause flaky or inconsistent test results at some point in the future.
So even if we decide to go with a global install for now, I'd suggest at least documenting that in the README to make it clear for everyone. That way, we avoid confusion if we have it explicitly listed in the prerequisite steps.
Description
Adds Playwright E2E tests. Also enables CI E2E tests for PRs.
See README.md for more details.
Notes