-
Notifications
You must be signed in to change notification settings - Fork 704
feat(amazonq): package.json npm scripts for build issues with vscode-extension-tester (VET) #7666
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): package.json npm scripts for build issues with vscode-extension-tester (VET) #7666
Conversation
… that can automatically pull the correct vsix version name
|
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, great start!
package.json
Outdated
@@ -28,6 +28,8 @@ | |||
"test": "npm run test -w packages/ --if-present", | |||
"testWeb": "npm run testWeb -w packages/ --if-present", | |||
"testE2E": "npm run testE2E -w packages/ --if-present", | |||
"setupE2E": "cd packages/amazonq && npm run package > ../../vsix.log 2>&1 && cd ../.. && node_modules/.bin/extest install-vsix --vsix_file amazon-q-vscode-$(node scripts/get-vsix-version.js).vsix", |
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 it worth making this setupUI
to match the testUI
below?
packages/amazonq/.vscodeignore
Outdated
@@ -0,0 +1,32 @@ | |||
# Source files |
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 don't think you meant to commit this file.
packages/amazonq/package.json
Outdated
@@ -2,7 +2,7 @@ | |||
"name": "amazon-q-vscode", | |||
"displayName": "Amazon Q", | |||
"description": "The most capable generative AI-powered assistant for building, operating, and transforming software, with advanced capabilities for managing data and AI", | |||
"version": "1.83.0-SNAPSHOT", |
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 believe this must stay as SNAPSHOT. Is there we reason we want to change it here?
packages/amazonq/package.json
Outdated
@@ -13,6 +13,7 @@ | |||
"type": "git", | |||
"url": "https://github.com/aws/aws-toolkit-vscode" | |||
}, | |||
"homepage": "https://github.com/aws/aws-toolkit-vscode", |
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 this change still necessary since we are no longer running the setup-test
command?
packages/amazonq/package.json
Outdated
@@ -1322,26 +1323,40 @@ | |||
"fontCharacter": "\\f1de" | |||
} | |||
}, | |||
"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.
I think you'll have to rebase to remove these changes. Solved this issue with Laura, so she is an expert now :)
// need this timeout because Amazon Q takes awhile to load | ||
this.timeout(30000) | ||
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.
I think these changes are not meant to be part of this CR? They look like the ones Laura made in the other open one.
packages/amazonq/vsix.log
Outdated
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 think this was accidentally committed.
packages/toolkit/package.json
Outdated
@@ -2,7 +2,7 @@ | |||
"name": "aws-toolkit-vscode", | |||
"displayName": "AWS Toolkit", | |||
"description": "Including CodeCatalyst, Infrastructure Composer, and support for Lambda, S3, CloudWatch Logs, CloudFormation, and many other services.", | |||
"version": "3.69.0-SNAPSHOT", | |||
"version": "3.69.0-g8500b1e", |
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 as previous comment about keeping this as snapshot.
scripts/get-vsix-version.js
Outdated
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.
out of curiosity, could we do this via CLI? I imagine we could use grep somehow. It might be somewhat complex, but I bet AI could write it.
The advantage of using a CLI command is that we avoid adding more files, and use already tested/maintained tools instead of creating our own.
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.
Fixed it!
package.json
Outdated
@@ -28,6 +28,8 @@ | |||
"test": "npm run test -w packages/ --if-present", | |||
"testWeb": "npm run testWeb -w packages/ --if-present", | |||
"testE2E": "npm run testE2E -w packages/ --if-present", | |||
"setupE2E": "cd packages/amazonq && npm run package > ../../vsix.log 2>&1 && cd ../.. && node_modules/.bin/extest install-vsix --vsix_file amazon-q-vscode-$(node scripts/get-vsix-version.js).vsix", |
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 we can do this without creating a temporary file (i.e. keeping the contents in memory).
If we do need the file, then we have to worry about conflicts with existing files as well as cleaning it up properly.
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.
Good idea, I just implemented that in the new PR
Looks like you may have to rebase anyway as I see some merge conflicts in the UI. |
…guements, fixed PR conflicts
## Changes This is the initial commit for UI Tests using the framework vscode-extension-tester. It instantiates the VSCode instance which opens Amazon Q and goes through some basic flows to login. This is done using the [VSCode-Extension-Tester Framework](https://github.com/redhat-developer/vscode-extension-tester/wiki/). (Note: It is missing full authentication and stops working once Amazon Q needs to open the browser). **To run this test** 1. First compile the test files `npm run testCompile` 2. Then run the test setup `node_modules/.bin/extest setup-tests` 2. Then run the script (NOTE: this js file should be generated when you compile the test files) `node_modules/.bin/extest run-tests aws-toolkit-vscode/packages/amazonq/dist/test/e2e/amazonq/VET.test.js` For information why each dependency / webpack change is needed, please see [here](https://quip-amazon.com/lCuBAOGibHzm/UI-Tests-PR-Explanation). Demo Video: https://github.com/user-attachments/assets/f1c6e59f-d4e3-4ae0-a164-1da389ec1339 --- - 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.
… that can automatically pull the correct vsix version name
…guements, fixed PR conflicts
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.
wooo, improvements!
scripts/package.ts
Outdated
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.
looks like its trying to format this file. I think you'll have to checkout the version from master and commit without the preommit hook. Something like:
git checkout origin/master scripts/package.ts
git commit -m 'fix: ....' -n
I believe the -n flag forces it to not run the linting on that commit.
packages/amazonq/package.json
Outdated
@@ -847,6 +865,24 @@ | |||
} | |||
], | |||
"keybindings": [ | |||
{ |
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 think you can do the same thing as below to not include this file since it doesn't look like these are necessary changes.
@@ -28,6 +28,10 @@ | |||
"test": "npm run test -w packages/ --if-present", | |||
"testWeb": "npm run testWeb -w packages/ --if-present", | |||
"testE2E": "npm run testE2E -w packages/ --if-present", | |||
"test:ui:prepare": "./node_modules/.bin/extest get-vscode -s ~/.vscode-test-resources -n && extest get-chromedriver -s ~/.vscode-test-resources -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.
its not entirely clear to me when each of these commands should be run and how they are distinguished. Do you mind updating the PR description to give some more context here?
Ex. What order are they run? Why are they split up? etc.
I suspect they are very good reasons for this, but looking at the commands its not obvious.
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.
Will do, not sure how I missed that, thanks!
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.
woohoo!
Looks like there might be a build issue causing the package to stall? Once thats resolved this LGTM. |
…extension-tester (VET) (aws#7666) Users running our new E2E UI testing suite have to run a plethora of commands in order to setup and run our test suite each time their testing code may change or a new workspace in VSCode is instantiated. This [document](https://quip-amazon.com/KfISA389hGty/Setup-Instructions-for-Amazon-Q-VET-UI-Tests) contains rationale. These commands are exact and can be easy to forget and small errors in the CLI commands lead to headaches with the setup. Additionally, we run into a problem when running `npm run package` where each VSIX installation will have a unique hash and VSCode version attached to the name, and due to the lack of CLI args for renaming our VSIX package, we must be able to reference the path to this VSIX installation on anyones local devices. Notably, many Vscode-extension-tester (VET) projects use "setup-tests" to attach a VSCode instance and a Chromedriver instance which is crucial for the functionality of VET; however, this leads to build issues due to the inherent nature of VET's "setup-tests" packaging its own VSIX automatically. In normal circumstances, this would be fine; however, due to our unique packaging structure of the amazonq directory, we require a package.ts script to package our directory properly. My solution encompasses adding additional scripts to the package.json file in the root directory of the aws-toolkit-vscode repository. These scripts must be run in a specific order in order for them to work correctly and must **all by run in the root directory.** Namely these npm scripts are: ``` "test:ui:prepare": "./node_modules/.bin/extest get-vscode -s ~/.vscode-test-resources -n && extest get-chromedriver -s ~/.vscode-test-resources -n", ``` This script essentially does the bulk of the previous "setup-tests", it installs a local VSCode instance and clears the previous cache and installs the chromdriver into the VSCode instance. This is stored in a vscode-test-resources directory in ones local User directory. ``` "test:ui:install": "cd packages/amazonq && npm run package 2>&1 | grep -o 'VSIX Version: [^ ]*' | cut -d' ' -f3 | xargs -I{} bash -c 'cd ../../ && ./node_modules/.bin/extest install-vsix -f amazon-q-vscode-{}.vsix -e packages/amazonq/test/e2e/amazonq/resources -s ~/.vscode-test-resources'", ``` This script installs our VSIX by going into the amazonq package and running our npm run package. This eliminates the need for any additional scripts by allowing for a CLI/RegEx command to be utilized to find the specific hash of the VSIX in our terminal output and input it within the install-vsix command for extest. ``` "test:ui:run": "npm run testCompile && ./node_modules/.bin/extest run-tests -s ~/.vscode-test-resources -e packages/amazonq/test/e2e/amazonq/resources packages/amazonq/dist/test/e2e/amazonq/VET.test.js", ``` This script runs our VET.test.js script and does the testCompile npm script that has already been built into the repo. This is because when we run "npm run package", our compiled tests within the dist directory are deleted. We must run testCompile after this command in order to properly compile our tests. ``` "test:ui": "npm run test:ui:prepare && npm run test:ui:install && npm run test:ui:run", ``` This script combines all of our scripts into a chronological order. --- - 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. --------- Co-authored-by: laura-codess <[email protected]>
…extension-tester (VET) (aws#7666) Users running our new E2E UI testing suite have to run a plethora of commands in order to setup and run our test suite each time their testing code may change or a new workspace in VSCode is instantiated. This [document](https://quip-amazon.com/KfISA389hGty/Setup-Instructions-for-Amazon-Q-VET-UI-Tests) contains rationale. These commands are exact and can be easy to forget and small errors in the CLI commands lead to headaches with the setup. Additionally, we run into a problem when running `npm run package` where each VSIX installation will have a unique hash and VSCode version attached to the name, and due to the lack of CLI args for renaming our VSIX package, we must be able to reference the path to this VSIX installation on anyones local devices. Notably, many Vscode-extension-tester (VET) projects use "setup-tests" to attach a VSCode instance and a Chromedriver instance which is crucial for the functionality of VET; however, this leads to build issues due to the inherent nature of VET's "setup-tests" packaging its own VSIX automatically. In normal circumstances, this would be fine; however, due to our unique packaging structure of the amazonq directory, we require a package.ts script to package our directory properly. My solution encompasses adding additional scripts to the package.json file in the root directory of the aws-toolkit-vscode repository. These scripts must be run in a specific order in order for them to work correctly and must **all by run in the root directory.** Namely these npm scripts are: ``` "test:ui:prepare": "./node_modules/.bin/extest get-vscode -s ~/.vscode-test-resources -n && extest get-chromedriver -s ~/.vscode-test-resources -n", ``` This script essentially does the bulk of the previous "setup-tests", it installs a local VSCode instance and clears the previous cache and installs the chromdriver into the VSCode instance. This is stored in a vscode-test-resources directory in ones local User directory. ``` "test:ui:install": "cd packages/amazonq && npm run package 2>&1 | grep -o 'VSIX Version: [^ ]*' | cut -d' ' -f3 | xargs -I{} bash -c 'cd ../../ && ./node_modules/.bin/extest install-vsix -f amazon-q-vscode-{}.vsix -e packages/amazonq/test/e2e/amazonq/resources -s ~/.vscode-test-resources'", ``` This script installs our VSIX by going into the amazonq package and running our npm run package. This eliminates the need for any additional scripts by allowing for a CLI/RegEx command to be utilized to find the specific hash of the VSIX in our terminal output and input it within the install-vsix command for extest. ``` "test:ui:run": "npm run testCompile && ./node_modules/.bin/extest run-tests -s ~/.vscode-test-resources -e packages/amazonq/test/e2e/amazonq/resources packages/amazonq/dist/test/e2e/amazonq/VET.test.js", ``` This script runs our VET.test.js script and does the testCompile npm script that has already been built into the repo. This is because when we run "npm run package", our compiled tests within the dist directory are deleted. We must run testCompile after this command in order to properly compile our tests. ``` "test:ui": "npm run test:ui:prepare && npm run test:ui:install && npm run test:ui:run", ``` This script combines all of our scripts into a chronological order. --- - 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. --------- Co-authored-by: laura-codess <[email protected]>
Problem
Users running our new E2E UI testing suite have to run a plethora of commands in order to setup and run our test suite each time their testing code may change or a new workspace in VSCode is instantiated. This document contains rationale. These commands are exact and can be easy to forget and small errors in the CLI commands lead to headaches with the setup. Additionally, we run into a problem when running
npm run package
where each VSIX installation will have a unique hash and VSCode version attached to the name, and due to the lack of CLI args for renaming our VSIX package, we must be able to reference the path to this VSIX installation on anyones local devices. Notably, many Vscode-extension-tester (VET) projects use "setup-tests" to attach a VSCode instance and a Chromedriver instance which is crucial for the functionality of VET; however, this leads to build issues due to the inherent nature of VET's "setup-tests" packaging its own VSIX automatically. In normal circumstances, this would be fine; however, due to our unique packaging structure of the amazonq directory, we require a package.ts script to package our directory properly.Solution
My solution encompasses adding additional scripts to the package.json file in the root directory of the aws-toolkit-vscode repository. These scripts must be run in a specific order in order for them to work correctly and must all by run in the root directory. Namely these npm scripts are:
This script essentially does the bulk of the previous "setup-tests", it installs a local VSCode instance and clears the previous cache and installs the chromdriver into the VSCode instance. This is stored in a vscode-test-resources directory in ones local User directory.
This script installs our VSIX by going into the amazonq package and running our npm run package. This eliminates the need for any additional scripts by allowing for a CLI/RegEx command to be utilized to find the specific hash of the VSIX in our terminal output and input it within the install-vsix command for extest.
This script runs our VET.test.js script and does the testCompile npm script that has already been built into the repo. This is because when we run "npm run package", our compiled tests within the dist directory are deleted. We must run testCompile after this command in order to properly compile our tests.
This script combines all of our scripts into a chronological order.
feature/x
branches will not be squash-merged at release time.