-
Notifications
You must be signed in to change notification settings - Fork 150
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
Run VSCode extension's tests on top of che-theia #864
Conversation
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
Signed-off-by: svor <[email protected]>
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 work @svor 🎉
I left some comments, see them inline
|
||
sudo pip install yq | ||
|
||
CHANGED_LINES=$(git diff -U0 HEAD "$(git merge-base HEAD origin/master)" che-theia-plugins.yaml | grep @@ | cut -d ' ' -f 3 | sed 's/+//') |
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 it would be good to extract out the arguments for git diff
into arguments for the script. That way it's not hardcoded in the script, but takes input from the GitHub action job.
For example:
https://github.com/eclipse/che-plugin-registry/blob/master/.ci/sidecar-build-publish.sh#L16
https://github.com/eclipse/che-plugin-registry/blob/master/.github/workflows/sidecar-build-push.yaml#L48
docker pull quay.io/eclipse/che-plugin-registry:nightly | ||
BUILDER=docker ./build.sh --tag vscode-extension | ||
# save locally built image | ||
docker save -o docker-image.tar quay.io/eclipse/che-plugin-registry:vscode-extension |
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's try a better name than vscode-extension
, maybe vscode-extension-test
or something similar?
|
||
function installDeps() { | ||
sudo pip install selenium | ||
wget https://github.com/mozilla/geckodriver/releases/download/v0.29.0/geckodriver-v0.29.0-linux64.tar.gz |
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.
Please create an variable for the geckodriver version, and use that variable in the URL. This makes it easier to read and update.
set -e | ||
|
||
function installDeps() { | ||
sudo pip install selenium |
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.
It might be better to create a user level virtual environment.
|
||
NEW_USER="admin" | ||
|
||
browser = webdriver.Firefox(options=options, executable_path="/usr/local/bin/geckodriver") | ||
wait = WebDriverWait(browser, 30) | ||
print (sys.argv[1]) |
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 a print statement left over from development 😄
image: 'quay.io/eclipse/che-theia-dev:next' | ||
alias: che-dev | ||
- type: chePlugin | ||
reference: 'https://raw.githubusercontent.com/svor/che-vscode-extension-tests/main/meta.yaml' |
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 central place we can put this, i.e. not your fork?
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.
@ericwill I think we can move this extension into che-incubator, but when eclipse-che/che#19172 will be resolved. I'm still working on this extension.
git status | ||
} | ||
|
||
function prepareDevfile() { |
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's move all the logic of this file to a github action
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.
@benoitf do you mean to create more steps in the action and each step will execute some part of this script?
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 was not clear enough
today you're using that script by doing:
name: Run VSCode extension's tests
if: env.EXTENSION_REPO != ''
run: |
./.ci/run-extension-tests.sh
but if you turn it to a github action, then we could just invoke it by calling
if: env.EXTENSION_REPO != ''
uses: che-incubator/vscode-extension-tests@next
(so plug-in registry is only a client of the action hosted elsewhere)
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.
and with github actions it's easy to get unit tests / code coverage / etc.
@svor: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
outdated |
What does this PR do?
This PR contains an action VSCode Extension's Tests to run tests from VSCode extension on top of Che-Theia if it was changed in
che-theia-plugins.yaml
.VSCode Extension's Tests workflow:
https://github.com/svor/che-plugin-registry/actions/workflows/vscode-extension-tests-check.yaml
The first step is Checkout. It clones the repository and makes checkout to the working branch.
Step 2 is Get changes. This step executes
./ci/get-changed-plugin.sh
. In this script we check what exactly was changed inche-theia-plugins.yaml
. If this file doesn't contain changes on lines with keyrevision
- it means that none extension is going to be updated or added and in this way we setEXTENSION_REPO
environment variable as empty and all next action's steps will be skipped (action should be finished). In another way, when some value of revision was changed, we find all information about the repository of changed extension (url and a revision).Step 3 is Build image. It builds
che-plugin-registry
image with all updates. It is needed to run the workspace with updated/added plugin.Step 4 is Start minikube. Needs to setup minikube
Step 5 is Load image. It loads che-plugin registry image into docker registry
Step 6 is Deploy Eclipse Che. This step uses
che-incubator/che-deploy-action@next
action to deploy Che with custom che-plugin-registry image into minikube.Step 7 is Run VSCode extension's tests. This step executes
./ci/run-extension-tests.sh
. The script has a few steps:url
andrevision
from step add cico script and rhel dockerfile #2 we clone extension repository into the/tmp/projects/
folder and do checkout to the branchyarn install
andyarn compile
--- I'm not sure if it will work for all extension).ci/templates/extension-tests-devfile.yaml
and it looks likeSo, this step builds id of tested extension by analysing
che-theia-plugins.yaml
. If the changed plugin contains id we just take it, if not we build it from extension'spackage.jon
aspublisher/name/latest
. When we have the id we need to replace @ with it in the devfile.How are tests running.
The workspace contains
che-vscode-extension-tests
plug-in (it locates in local repository https://github.com/svor/che-vscode-extension-tests) it is included in the devfile:When the workspace was started, this plugin automatically finds all *.test.js files in
/projects
folder inside the workspace and run them one by one via mocha framework.For now tests are failed because each test needs to execute vscode.extensions.getExtension(id) to activate an extension and it gets
Cannot read property 'getExtension' of undefined
. There is an issue to investigate this problem: eclipse-che/che#19172Screenshot/screencast of this PR
Some details of execution is here https://github.com/svor/che-plugin-registry/runs/1979560988?check_suite_focus=true
What issues does this PR fix or reference?
eclipse-che/che#18219
How to test this PR?
PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or reference
andHow to test this PR
completedReviewers
Reviewers, please comment how you tested the PR when approving it.