[code-infra] Playwright 1.49 #15493

JCQuintas · 2024-11-19T16:14:41Z

Closes #13672.
Closes #14948.

I have followed (at least) the PR section of the contributing guide.

mui-bot · 2024-11-19T16:18:43Z

Deploy preview: https://deploy-preview-15493--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 017ff19

This reverts commit 3d1d77d.

This reverts commit a0874a0.

github-actions · 2024-11-20T16:52:57Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

…ht-1.49

JCQuintas · 2024-11-26T15:25:37Z

@LukasTy I've divided the e2e tests... they are interfering with eachother a lot due to async bleeding everywhere.

The DataGrid runs independently while the other tests are bundled together.
When DataGrid runs with the other tests, a lot of timeouts and async errors happen with DatePickers and TreeView.

This is probably similar reason why the tests are failing on vitest as well. IMO we should look into adding eslint rules to force everyone to use the correct test patterns, though it is a bit difficult cause I think some of the issues come from core's test utils as well 😢

github-actions · 2024-11-28T11:31:22Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

…ht-1.49

github-actions · 2024-11-29T12:04:13Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

LukasTy

Looks good, leaving some comments. 😉

LukasTy · 2024-11-29T12:51:01Z

packages/x-data-grid/src/tests/keyboard.DataGrid.test.tsx

      const virtualScroller = document.querySelector<HTMLElement>('.MuiDataGrid-virtualScroller')!;
      expect(virtualScroller.scrollLeft).to.equal(0);
-      fireEvent.keyDown(document.activeElement!, { key: 'ArrowRight' });
+      // We then need to move up to the group header, then right to the first named column
+      await user.keyboard('{ArrowUp}{ArrowUp}{ArrowRight}');


This seems different to the previous behavior on L516. 🤔
Did you have problems with?

await act(() => getColumnHeaderCell(0, 0).focus());

The same question applies to the whole file.

🤔 It should probably work fine.

My main grip is that using .focus() is not really how the user uses the keyboard. So if you want to test that your behaviour is working properly, you should emulate the entire action.

I would leave this call for @mui/xgrid, but maybe we can create an alternative getColumnHeaderCell or focusColumnHeaderCell function, which would click on the respective cell? 🤔

I didn't add clicking on the cell itself cause I don't know if that causes other side effects

Eg: if we have selectable rows and we click on column 0, then it "selects" the row.

Focusing through keyboard interactions seemed more safe.

The main difference on why it has more "keypresses" than before is because the default focus goes to the "effective" first entry on the header if we press tab, but these tests are testing the column groups, which means they were first focusing on the first ever element on the header getColumnHeaderCell(0, 0), rather than the element that gets the focus first, which in this case would be getColumnHeaderCell(0, 2)

So, this means

Previous getColumnHeaderCell(0, 0)

H - Header
C - Content Cell

H00 H10 H20

H01 H11 H21

H02 H12 H22

--- --- ---

C00 C10 C20

[Tab] -> getColumnHeaderCell(0, 2)

H00 H10 H20

H01 H11 H21

H02 H12 H22

--- --- ---

C00 C10 C20

I simply moved the focus back to the getColumnHeaderCell(0, 0) from the original tests.

Now, it might be that the element at 0,2 shouldn't get focus on tab, which would be another problem unrelated to this PR IMO 😅

But the tests are comparable between themselves and the assertions didn't change.

LukasTy · 2024-11-29T12:52:34Z

packages/x-data-grid/src/tests/rowSelection.DataGrid.test.tsx

-        expect(document.querySelectorAll('.MuiTouchRipple-rippleVisible')).to.have.length(1);
-      });
-    });
+    // Skip on everything as this is failing on all environments


Has this started happening only with the bump of playwright?

Yeah, it was happening since the first run of this PR, but it seems to be fixed if I revert it now... at least locally. I guess dividing the tests helped with the inconsistencies...

Seems to be consistently failing in CI and local ubuntu though... OSX seems fine, though that is of no use for our CI, so I just ignored it again 🙃

Left a new comment on this topic: #15493 (comment)

test/regressions/index.test.ts

LukasTy · 2024-12-02T13:12:24Z

packages/x-data-grid/src/tests/keyboard.DataGrid.test.tsx

      const virtualScroller = document.querySelector<HTMLElement>('.MuiDataGrid-virtualScroller')!;
      expect(virtualScroller.scrollLeft).to.equal(0);
-      fireEvent.keyDown(document.activeElement!, { key: 'ArrowRight' });
+      // We then need to move up to the group header, then right to the first named column
+      await user.keyboard('{ArrowUp}{ArrowUp}{ArrowRight}');


I would leave this call for @mui/xgrid, but maybe we can create an alternative getColumnHeaderCell or focusColumnHeaderCell function, which would click on the respective cell? 🤔

LukasTy · 2024-12-02T13:16:46Z

packages/x-data-grid/src/tests/rowSelection.DataGrid.test.tsx

-      });
-    });
-  });
+  // Skip on everything as this is failing on all environments on ubuntu/CI


have you considered rewriting it with async syntax, not using fake timers, but trying to rely on DOM assertions, like checking if the new item has a ripple effect and only one DOM element has a ripple effect..? 🤔

Yeah, that was the previously commented code. I only in this last commit reverted the ripple logic to the original one. 815810d

…ht-1.49

LukasTy

LGTM. 👌
Thank you for the massive effort on this one. 💯 🚀
And finding the root cause, why screenshot taking of print preview was failing. 🙈

Most Argos diffs make sense, possibly due to a big jump in browser versions.
But on instance jumps to me: https://app.argos-ci.com/mui/mui-x/builds/27473/122741853
Could this be a fluke? Or a side effect of the moving the mouse?
Maybe with the updates to browser or playwright the initial mouse position is in the middle of the viewport and moving it to 0, 0 causes the controlled highlight to be removed? 🤔

JCQuintas · 2024-12-02T16:18:02Z

LGTM. 👌 Thank you for the massive effort on this one. 💯 🚀 And finding the root cause, why screenshot taking of print preview was failing. 🙈

Most Argos diffs make sense, possibly due to a big jump in browser versions. But on instance jumps to me: https://app.argos-ci.com/mui/mui-x/builds/27473/122741853 Could this be a fluke? Or a side effect of the moving the mouse? Maybe with the updates to browser or playwright the initial mouse position is in the middle of the viewport and moving it to 0, 0 causes the controlled highlight to be removed? 🤔

Oh yeah, that was why I moved the cursor to the top on the first place IIRC 🤣

JCQuintas added 2 commits November 19, 2024 17:12

use latest versions

0e78492

install ffmpeg

1a8b8ce

JCQuintas added scope: code-infra Specific to the core-infra product test labels Nov 19, 2024

JCQuintas self-assigned this Nov 19, 2024

JCQuintas added 19 commits November 19, 2024 17:32

try removing changes

20feb46

Merge branch 'master' into playwright-1.49

aa80576

revert aliasing

36fe4ec

try headless

0b048d1

improve headless mode

316e231

add comment

57f49bb

try chrome channel

501774d

revert channel

bc67af0

try ss options

4d05ebc

add chart test to time sensitive

06ed4ee

try border box

3d1d77d

Revert "try border box"

8ad7c1c

This reverts commit 3d1d77d.

move mouse as first thing

48f62fe

try wait for font

a0874a0

Revert "try wait for font"

9f5eb1c

This reverts commit a0874a0.

try scrolling hack

73a1a54

remove

56fbd76

add preconnect

f3afe39

add docker image to renovate

ae48c8d

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 20, 2024

LukasTy mentioned this pull request Nov 22, 2024

Lock file maintenance #11894

Merged

1 task

Merge commit '1ed1f8c0dcc4e46cea9d245696617fdd679bf01b' into playwrig…

752ecfa

…ht-1.49

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 25, 2024

revert changes

4ab0284

JCQuintas marked this pull request as ready for review November 26, 2024 15:21

JCQuintas requested a review from LukasTy November 26, 2024 16:17

Merge branch 'master' into playwright-1.49

be5cf55

JCQuintas mentioned this pull request Nov 27, 2024

Bump Playwright to 1.49.0 mui/material-ui#44527

Open

1 task

remove extra props

f2a7f25

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 28, 2024

Merge commit 'e465d2f0938b085385a761139e5d3ac2fa7e50a2' into playwrig…

28dc2a3

…ht-1.49

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 28, 2024

Merge commit '73bf12315f63d2f7210e2d368a258e86a5f97e58' into playwrig…

f1a9549

…ht-1.49

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 29, 2024

LukasTy reviewed Nov 29, 2024

View reviewed changes

JCQuintas added 4 commits December 2, 2024 11:14

Revert changes

fde716a

try removing arg

d235c1b

Revert ripple test

815810d

Ignore ripple test

2521817

LukasTy reviewed Dec 2, 2024

View reviewed changes

Merge commit '82d46ab329a11bf9308e8a1cf57786ac0cec37ad' into playwrig…

9327762

…ht-1.49

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 2, 2024

fix test

15c6cfe

JCQuintas requested a review from a team December 2, 2024 14:58

LukasTy approved these changes Dec 2, 2024

View reviewed changes

revert mouse move

017ff19

JCQuintas enabled auto-merge (squash) December 2, 2024 16:33

JCQuintas merged commit 01fbad7 into mui:master Dec 2, 2024
16 checks passed

JCQuintas deleted the playwright-1.49 branch December 2, 2024 16:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[code-infra] Playwright 1.49 #15493

[code-infra] Playwright 1.49 #15493

JCQuintas commented Nov 19, 2024 •

edited by LukasTy

Loading

mui-bot commented Nov 19, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024

JCQuintas commented Nov 26, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 29, 2024

LukasTy left a comment

LukasTy Nov 29, 2024

JCQuintas Dec 2, 2024

LukasTy Dec 2, 2024

JCQuintas Dec 2, 2024 •

edited

Loading

LukasTy Nov 29, 2024

JCQuintas Dec 2, 2024

JCQuintas Dec 2, 2024

LukasTy Dec 2, 2024

LukasTy Dec 2, 2024

LukasTy Dec 2, 2024

JCQuintas Dec 2, 2024

LukasTy left a comment

JCQuintas commented Dec 2, 2024

[code-infra] Playwright 1.49 #15493

[code-infra] Playwright 1.49 #15493

Conversation

JCQuintas commented Nov 19, 2024 • edited by LukasTy Loading

mui-bot commented Nov 19, 2024 • edited Loading

github-actions bot commented Nov 20, 2024

JCQuintas commented Nov 26, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 29, 2024

LukasTy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JCQuintas Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukasTy left a comment

Choose a reason for hiding this comment

JCQuintas commented Dec 2, 2024

JCQuintas commented Nov 19, 2024 •

edited by LukasTy

Loading

mui-bot commented Nov 19, 2024 •

edited

Loading

JCQuintas Dec 2, 2024 •

edited

Loading