playwright tests#829
Conversation
patrick-skowronek
left a comment
patrick-skowronek
left a comment
There was a problem hiding this comment.
The e2e Test look good, but there are some things in the tests. Please add documentation for the e2e test, as a test plan. Also add a section to the lens book what and how changes should be documented.
| with: | ||
| node-version: latest | ||
| - run: npm ci | ||
| - run: npx playwright install chromium --with-deps |
There was a problem hiding this comment.
This should be using --only-shell to only install headless
| - run: npx vite build | ||
| - run: npx vitest | ||
| - run: npx vite build --config vite.config.demo.js | ||
| - run: npx playwright install chromium --with-deps |
There was a problem hiding this comment.
This should be using --only-shell to only install headless
| - run: npx vite build | ||
| - run: npx vitest | ||
| - run: npx vite build --config vite.config.demo.js | ||
| - run: npx playwright install chromium --with-deps |
There was a problem hiding this comment.
Should the default be chrome? Maybe it would make sense for pull request to also test in firefox and webkit(safari)
| }); | ||
| }); | ||
|
|
||
| test("getAst: empty queryStore returns top-level OR with no children", () => { |
There was a problem hiding this comment.
We should document what is the default for an empty query store. For me an empty or is fine. But in the future where it could start with a "and" we should be prepared.
| }); | ||
|
|
||
| test("getAst: empty queryStore returns top-level OR with no children", () => { | ||
| setQueryStoreFromAst({ operand: "OR", children: [] }); |
There was a problem hiding this comment.
This function is deprecated and should be tested. Instead please test the QueryStore2AST
| [makeItem("First name", [{ name: "Olaf", value: "Olaf" }])], | ||
| ]); | ||
| const result = getHumanReadableQueryAsFormattedString(); | ||
| // translate("query_info_header") → "Search ANY of the following groups" |
There was a problem hiding this comment.
Why is this commented out? Testing translate should be done in the translate file.
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "status": "passed", | |||
There was a problem hiding this comment.
This should not be committed and be set in the .gitignore
| page, | ||
| }) => { | ||
| const catalogue = page.locator("lens-catalogue"); | ||
| // Expand the group first |
There was a problem hiding this comment.
Do we need the comments?
There was a problem hiding this comment.
Maybe change the file name. This tests mostly the searchbar/querystore functionality.
|
Whoof, adding a giant heap of vibe-coded integration tests just feels like the wrong move to me especially given that Lens must undergo major changes in the near future. This feels like just adding tests onto everything without consideration if this is actually something that is likely to break. I've just read a few tests (there are too many for me to read though) but the ones I've seen are kind of tightly coupled to the DOM structure. For example the tests for the query explain button seem basically to enforce the exact behavior and DOM structure of how the query explain button renders out the query right now. Any changes to the query explain button would require changing the tests accordingly. And I just can't see what regression this would actually catch. |
3 participants
Description
This PR introduces end-to-end testing with Playwright, adding specs that cover the core UI workflows including search bar autocomplete, query building (AND/OR logic), result display, catalogue interaction, language switching, and toast notifications.
Playwright is wired into both the standard CI linting pipeline and the release verification workflow, with support for running against either a local dev server or the live GitHub Pages demo.