-
Notifications
You must be signed in to change notification settings - Fork 575
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
chore: Serialise web and openfin e2e tests execution #2296
chore: Serialise web and openfin e2e tests execution #2296
Conversation
(auto-deploy) A deployment has been created for this Pull Request Preview linksAs part of the code review process, please ensure that you test against the following
PerformancePlease ensure that this PR does not degrade the performance of the UI. We should maintain a performance score of 95+. |
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.
Like you, I'd prefer if we could do these tests in parallel, especially as they are in a Pull Request workflow .. if this becomes onerous, we could consider bypassing the OpenFin tests, but make sure we run them on master (this would require a bit of gnarly logic in the branch workflow tho, so far from ideal)
@@ -148,7 +148,7 @@ test.describe("Credit", () => { | |||
|
|||
await sellSidePage.waitForSelector("text=New RFQ") | |||
|
|||
await sellSidePage.getByTestId("price-input").fill("100") | |||
await sellSidePage.getByTestId("price-input").pressSequentially("100") |
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 - are there any more lying around?
in some cases a .fill
will be appropriate, but maybe as a parallel test, to check what happens if people paste values in - can't think of one off-hand in RT, mind you .. RA perhaps?
(I am referring to the Playwright 1.38 update video)
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.
There is no explicit test validating how pasting a value in a text field behave, either in RT or RA. There are still few .fill
being used in RT. I will update them to .pressSequentially
in a subsequent PR
Serializing
web
->openfin
e2e tests on a PR basis to prevent intermittent concurrency issues that cause false negative results