-
Notifications
You must be signed in to change notification settings - Fork 93
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
E2E test: Positron editors POM #5845
Conversation
E2E Tests 🚀 ? |
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!
if (process.platform === 'darwin') { | ||
await this.code.driver.page.keyboard.press('Meta+N'); | ||
} else { | ||
await this.code.dispatchKeybinding('Control+N'); |
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.
Did you miss this one?
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.
oops. yup
|
||
let error: unknown | undefined = undefined; | ||
let retries = 0; | ||
while (retries < 10) { |
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.
Could this be the "expect" toPass() instead?
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.
yeah, i do like that the basic loops can capture the error and surface it, but this could definitely be a toPass(). i could go either way. it is a little annoying that toPass blocks always just fail on a 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.
i will go ahead and convert.... toPass is more concise
Replace MS editors POM with one based solely on Playwright
QA Notes
All tests should pass.