Skip to content

Use the correct environment variable in integration tests #994

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

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

aeisenberg
Copy link
Contributor

@aeisenberg aeisenberg commented Nov 3, 2021

Also, fix documentation and launch configs to specify the correct and
complete set of environment variables we should be using.

This should fix the failing integration tests on our internal repository.

Replace this with a description of the changes your pull request makes.

Checklist

  • [n/a] CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • [n/a Issues have been created for any UI or other user-facing changes made by this pull request.
  • [n/a] @github/docs-content-codeql has been cc'd in all issues for UI or other user-facing changes made by this pull request.

@aeisenberg aeisenberg requested a review from a team as a code owner November 3, 2021 20:38
CONTRIBUTING.md Outdated
Ensure the `CODEQL_PATH` environment variable is set to point to the `codeql` cli executable.

Outside of vscode, run:
Unit tests and many integration tests do not Outside of vscode, run:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Unit tests and many integration tests do not Outside of vscode, run:
Unit tests and many integration tests do not require a copy of the CodeQL CLI. Outside of vscode, run:

@@ -122,13 +122,12 @@ jobs:
working-directory: extensions/ql-vscode
if: matrix.os == 'ubuntu-latest'
run: |
CODEQL_PATH=$GITHUB_WORKSPACE/codeql-home/codeql/codeql npm run test
Copy link
Contributor

Choose a reason for hiding this comment

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

So these tests are not using the CLI? Should we remove it from being installed in the step above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They may have at one point in the past, but not any more. I will remove the section.

Also, fix documentation and launch configs to specify the correct and
complete set of environment variables we should be using.
@adityasharad adityasharad enabled auto-merge (squash) November 3, 2021 22:58
@adityasharad adityasharad merged commit 0ef635b into main Nov 3, 2021
@adityasharad adityasharad deleted the aeisenberg/env-var branch November 3, 2021 23:04
@adityasharad
Copy link
Contributor

Updated the branch protection rules for main: we now require Test (ubuntu-latest) and Test (windows-latest) rather than the {stable,nightly} variants of each.

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