Skip to content
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: GitHub actions CI #47

Merged
merged 10 commits into from
Aug 17, 2023
Merged

feat: GitHub actions CI #47

merged 10 commits into from
Aug 17, 2023

Conversation

meniRoy
Copy link
Contributor

@meniRoy meniRoy commented Aug 15, 2023

run integration tests in GitHub CI actions.

I encountered an encryption error during the setup: Error while encrypting the text provided to safeStorage.encryptString. Encryption is not available.
To address this, I've added the --password-store=basic argument to the launch parameters of @vscode/test-electron, as suggested here

@meniRoy
Copy link
Contributor Author

meniRoy commented Aug 15, 2023

Copy link

@sagilaufer1992 sagilaufer1992 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why to update the lock file, and I'm not familiar with the command you used for running the tests.
address these questions, and the rest LGTM

.github/workflows/test.yml Show resolved Hide resolved
pnpm-lock.yaml Outdated

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why updating the lock file? if the package json didn't update, is there a good reason to update it for this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I updated the lock file is due to an error I encountered when executing pnpm install within our GitHub Actions workflow.
The error message:

Run pnpm install
 ERR_PNPM_FROZEN_LOCKFILE_WITH_OUTDATED_LOCKFILE  Cannot perform a frozen installation because the version of the lockfile is incompatible with this version of pnpm

Try either:
1. Aligning the version of pnpm that generated the lockfile with the version that installs from it, or
2. Migrating the lockfile so that it is compatible with the newer version of pnpm, or
3. Using "pnpm install --no-frozen-lockfile".
Note that in CI environments, this setting is enabled by default.
Error: Process completed with exit code 1.

https://github.com/env0/vscode-env0/actions/runs/5870964165/job/15919142107

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like the installation of pnpm installed a different version than before.
you can see in our main repo, we expect to use a specific version of pnpm, specified in the package json under "engines".
make the GH action install a SPECIFIC pnpm version, that still corresponds to this lockfile, so we won't fave such issues later on in another pnpm upgrade

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meniRoy meniRoy requested a review from sagilaufer1992 August 16, 2023 12:44
Copy link

@sagilaufer1992 sagilaufer1992 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve, but please, if the pnpm need to be a dependency, add it, and if the lock file doesn't need to be changed, revert it

pnpm-lock.yaml Outdated

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we revert the pnpm lock? or is it required now that we have pnpm as an engine?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -5,7 +5,8 @@
"description": "",
"version": "0.0.1",
"engines": {
"vscode": "^1.71.0"
"vscode": "^1.71.0",
"pnpm": "6.35.1"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't it be a dev-dependency now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pnpm is not a dev dependency its installed globally with fixed version here

@meniRoy meniRoy merged commit bb8b233 into main Aug 17, 2023
@meniRoy meniRoy deleted the feat--GH-action-CI-workflow branch August 17, 2023 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants