-
Notifications
You must be signed in to change notification settings - Fork 731
feat(amazonq): add shortcut test for UI E2E Tests #7863
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
Conversation
|
|
Looks like CI is blocked: |
eslint
|
|
||
| describe('Amazon Q Shortcut Functionality', function () { | ||
| // this timeout is the general timeout for the entire test suite | ||
| this.timeout(150000) |
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.
can we centralize this to one place?
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.
TODO: once all PRs are in, go into all files and create a constant.ts file to manage repeated code. @surajrdy-aws
| await closeAllTabs(webviewView) | ||
| await clearChat(webviewView) | ||
| }) | ||
| it('Keybind Check', async () => { |
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.
same comment as other PR for the test titles.
| it('Keybind Check', async () => { | ||
| const driver = webviewView.getDriver() | ||
| // Open Command Palette | ||
| await pressShortcut(driver, Key.COMMAND, Key.SHIFT, 'p') |
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.
does VET not have native support for opening the command palette? Somewhat surprised by that.
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.
They do, but for these tests, they specifically wanted to test Keybinds on the computer as short cuts. I wonder if just using the native VET support is any different in the testing sense?
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.
interesting. Does VSC allow for users to overwrite key binds? If so would this test always fail on their machine?
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.
Yes I believe they do allow for users to overwrite key binds. The tests would then fail on the machine. I think generally this test is not extremely robust, but not sure how else to tackle it since some of the test descriptions explicitly asked for keybinds to be used. What are your thoughts on it?
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.
Yea, I guess this is fine for now. Could be worth adding a comment, but don't see it as a blocker.
|
|
||
| const driver = webviewView.getDriver() | ||
| await pressShortcut(driver, Key.COMMAND, Key.ALT, 't') | ||
| // Clean Up Text |
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.
nit: idk if these comments are needed.
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.
Gotcha!
| if (!responseReceived) { | ||
| throw new Error('Chat response not received within timeout') | ||
| } | ||
| console.log('Chat response detected successfully') |
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.
you shouldn't need to log successful tests. This should be done automatically by mocha.
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.
Let me fix that!
| * Ctrl + Shift + T | await pressShortcut(driver, Key.CONTROL, Key.SHIFT, 't') | ||
| */ | ||
| export async function pressShortcut(driver: WebDriver, ...keys: (keyof typeof Key)[]): Promise<void> { | ||
| export async function pressShortcut(driver: WebDriver, ...keys: (string | keyof typeof Key)[]): Promise<void> { |
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.
Is there a way to describe the 'a' key as the specified type rather than the string? Or is this change necessary?
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, so to my understanding, the problem isn't actually the 'a' string but also the IKey variables which are declared as strings. For example:
COMMAND: string; // Apple command key
|
/retryBuilds |
Problem
We have not implemented the shortcut tests for our UI E2E Test suite with VSCode-Extension-Tester. Additionally, we face a small problem in our pressShortcut abstraction that throws a type error when trying to reference a Key command like
Key.COMMANDsince it is a IKey String.Solution
We have implemented the shortcut tests:
We have also fixed the pressShortcut abstraction with a small edit in the logic of accepted parameters.
NOTE: We have chosen to implement the Inline Shortcut within the Inline Tests even though it could be classified under Shorcuts.
feature/xbranches will not be squash-merged at release time.