[ZEPPELIN-6358] Remove anti-patterns of E2E and tidy test suite#5207
[ZEPPELIN-6358] Remove anti-patterns of E2E and tidy test suite#5207jongyoul wants to merge 2 commits intoapache:branch-0.12from
Conversation
Applied the [`e2e-reviewer`](https://github.com/dididy/e2e-skills) skill on the existing E2E suite. The skill does static analysis — it catches tests that can never actually fail, silent skips, swallowed errors in POM methods, that kind of thing. Findings and fixes: - `home-page-enhanced-functionality.spec.ts` was mostly duplicating `home-page-elements` and `home-page-note-operations` → deleted and merged - `toBeGreaterThanOrEqual(0)` and `toBeAttached()` on static elements were always passing → replaced with assertions that can fail - `if (isVisible) { expect() }` patterns silently skip when something breaks → removed or converted to `test.skip` - Several POM methods had `.catch(() => {})` with no comment → removed; kept the intentional ones and marked with `// JUSTIFIED:` - `document.querySelector` in `page.evaluate()` → swapped for Playwright locator API - Added `aria-label` / `data-testid` to action bar HTML; a few tests were breaking on DOM structure changes - Renamed a handful of tests whose names didn't match what they actually tested; dropped the ones that only called `toBeVisible()` Improvement Refactoring ZEPPELIN-6358 * Does the license files need to update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Closes apache#5180 from dididy/tidy-e2e. Signed-off-by: Jongyoul Lee <jongyoul@gmail.com> (cherry picked from commit 076676a)
The failures are occurring in the React layer of the micro-frontend, and they appear to be flaky. I’ll investigate the root cause and share findings and potential fixes in the draft. (If needed, I’ll follow up with an additional PR) Also, I noticed several audit issues in the zeppelin-react-related packages. I’ll create a separate PR to address them all at once. |
|
@dididy You can freely modiry and/or create a your own PR. I just wanted to adopt it to branch-0.12. Could you please help to handle it? 🙏 |
What's happeningThe test The WhyThis PR cherry-picks the E2E tidy commit from master, but the tests reference the React micro-frontend feature added in ZEPPELIN-6371. That commit was never cherry-picked to https://github.com/jongyoul/zeppelin/tree/ZEPPELIN-6358-e2e-tidy-branch-0.12/zeppelin-web-angular/projects Options
|
|
@dididy Thanks for the detailed analysis! I went with Option 2 and cherry-picked your commit a73137d to drop the React-specific tests, since ZEPPELIN-6371 is a large feature that shouldn't be pulled into branch-0.12 just to satisfy tests. |
|
I checked the same workflow(frontend/run-playwright-e2e-tests) in my fork, and it completed successfully: https://github.com/dididy/zeppelin/actions/runs/24250787247 It looks like this might be a transient issue with the CI environment. Could you please try re-running the job? |
|
I triggered a re-run for failed jobs. So let's take a look. |
3 participants
Summary
Test plan