-
Notifications
You must be signed in to change notification settings - Fork 746
feat(amazonq): adding a simple chat prompt test #7643
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
feat(amazonq): adding a simple chat prompt test #7643
Conversation
…as webpack.NormalModuleReplacementPlugin that needed to be there in order for the amazonq to compile
…he checks. have to commit to check since the build compiles with no errors
|
…e tests are done running
Hweinstock
left a comment
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 start! Especially on the timeouts!
| ``` | ||
|
|
||
| ## Language Server Debugging | ||
|
|
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.
are these automatically applied by the linter?
| } | ||
| }, | ||
| "aws-schemas-registry": { | ||
| "aws-sagemaker-code-editor": { |
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.
this seems unrelated to your changes?
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 this is unrelated to my changes, i think they appeared when i ran "npm install" when we were resolving the merge conflicts, should i delete them?
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.
We should we able to resolve this by rebasing. If not, we can manually take the changes out of the PR since I'm unsure of the side effects.
| await workbench.executeCommand('Amazon Q: Open Chat') | ||
|
|
||
| await new Promise((resolve) => setTimeout(resolve, 15000)) | ||
| // need this 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 know its somewhat of a mystery, but it could be worth investigating why we need these timeouts? Maybe we can find another example of someone running into the same issue and link that here.
|
|
||
| // AFTER AUTHENTICATION WE MUST RELOAD THE WEBVIEW BECAUSE MULTIPLE WEVIEWS CANNOT BE READ AT THE SAME TIME | ||
| const editorView = workbench.getEditorView() | ||
| console.log('editorview successfully created') |
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 assume these are just debugging logs? We should be careful of this becoming super noisy, but if its helpful we should keep them. I imagine a subset of these are helpful?
| for (const button of closeButtons) { | ||
| await button.click() | ||
| // small delay to ensure the tab closes properly | ||
| await new Promise((resolve) => setTimeout(resolve, 500)) |
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 we use waitForElement here?
| I can check how many elements there are in our specific conversation container. If there is 2 elements, | ||
| we can assume that the chat response has been generated. The challenge is, we must grab the latest | ||
| conversation container, as there can be multiple conversations in the webview. */ | ||
| async function waitForChatResponse(webview: WebviewView, timeout = 15000): Promise<boolean> { |
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, I like the use of a default here! It helps simplify the interface for the common case.
test.log
Outdated
| @@ -0,0 +1,108 @@ | |||
|
|
|||
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 accidentally recommit the test.log?
8cf315a to
5da6336
Compare
## Changes Adding the simple chat prompt test. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
## Changes Adding the simple chat prompt test. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
Changes
Adding the simple chat prompt test.
feature/xbranches will not be squash-merged at release time.