test(amazonq): E2E tests for /test #6431
Conversation
|
/runintegrationtests |
| * | ||
| * @returns `TextDocument`, or undefined if the file could not be found. | ||
| */ | ||
| export async function openDocument(filePath: string): Promise<vscode.TextDocument | undefined> { |
There was a problem hiding this comment.
i'm certain there are similar functions in one of the util modules, can they be used instead? if not, then this new function should live one of the shared modules, not deep in a test-only, q-only subfolder.
There was a problem hiding this comment.
We have functions that does the same with an uri, so maybe this can work:
const document = await vscode.workspace.openTextDocument(vscode.Uri.file(filePath))
There was a problem hiding this comment.
Sure, I will look into this
There was a problem hiding this comment.
I've removed the helper since:
- it was only being called twice
- future language test additions will be handled within existing loop
After testing, I found that using vscode.workspace.findFiles() and its proceeding logic was the most reliable in opening the file for testing
There was a problem hiding this comment.
I definitely wasn't suggesting to inline the retry logic into the tests. My suggestion was to lift the util function into /shared/.
I've removed the helper since:
it was only being called twice
future language test additions will be handled within existing loop
you mean for these particular tests. but if opening files is unreliable, then surely we need that logic in all tests that open files. again, we already have existing functions that do this:
You could update that existing function to (1) use findFiles() and (2) to use waitUntil, to make it (optionally) more reliable.
The point of adding tests is to improve system reliability. Sharing code also improves system reliability. Copy-pasting code everywhere means that some code will just have hidden bugs.
There was a problem hiding this comment.
Gotcha, I just updated to use the openTextDocument() function from the aws-core-vscode/shared module. At first, I didn't realize to use this, so I tried making my own helper. The pre-existing helper works well and is reliable. Let me know if I should clarify anything else
| }) | ||
|
|
||
| describe('Unsupported language', () => { | ||
| const { language, filePath } = unsupportedLanguages[0] |
There was a problem hiding this comment.
just curious why we are only checking the first element?
There was a problem hiding this comment.
Just want to check that for an unsupported language (no specific one in particular), it will redirect to the chat.
So whatever the first language in unsupportedLanguages array will be used
| tab.addChatMessage({ command: '/test' }) | ||
| await tab.waitForChatFinishesLoading() |
There was a problem hiding this comment.
nit: looks like all tests do this, can it be added to before()?
There was a problem hiding this comment.
It can definitely be added to beforeEach(). Although I feel that since the beforeEach() is already responsible for opening an active file, it might just become more bloated. If someone in the future looks at the test I feel it would be easier if it is called in each test individually. Let me know what you think
Problem
Solution
feature/xbranches will not be squash-merged at release time.