-
Notifications
You must be signed in to change notification settings - Fork 46
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
front: fix flaky test 012 and 006 #9877
Conversation
d85cf49
to
9cac7c4
Compare
Signed-off-by: maymanaf <[email protected]>
9cac7c4
to
541a9f2
Compare
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.
Nice PR! There's a slight redundancy with 8248abe which adds await page.waitForSelector(containerSelector, { state: 'visible' });
to the top of the scroll container function
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 ✅
I don't see any redundancy in this case, as I didn't modify the scroll function. Additionally, the use of waitForSelector shouldn't cause any issues since it doesn't rely on a fixed timeout. |
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
Yes, but you are waiting for the table to be visible before calling the scroll function at multiple points, and my change makes the scroll function wait for the component it should scroll to be visible, so we solved the same bug twice. It shouldn't pose any issue though. If the table is visible once, it should be visible twice =p |
This PR addresses the flakiness in tests 006 and 012.
The Playwright runner is very fast, which sometimes prevents the results from being updated in time. To resolve this, I added timeouts