Skip to content

Update unset environment variables PR check #1269

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.

Sign up for GitHub

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
angelapwen merged 11 commits into from
Sep 27, 2022
Merged

Update unset environment variables PR check #1269

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ab921b6
Only test Java for CLI v2.5+
angelapwen Sep 26, 2022
81e0392
Add explanation in PR check comment
angelapwen Sep 27, 2022
2bdcdfe
Improve bash code style
angelapwen Sep 27, 2022
50fd69b
Bash syntax fixups
angelapwen Sep 27, 2022
bf48a18
Fix typo
angelapwen Sep 27, 2022
d224372
Print variables for debugging
angelapwen Sep 27, 2022
8374a4e
Fix more bash typos
angelapwen Sep 27, 2022
8024938
Fix bash syntax bugs
angelapwen Sep 27, 2022
231835d
Set Actions error messages
angelapwen Sep 27, 2022
e161371
Add whitespace
angelapwen Sep 27, 2022
9d42ae7
Replace `*` matching in bash script
angelapwen Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 0 additions & 97 deletions .github/workflows/__unset-environment.yml
View file
Open in desktop

This file was deleted.

95 changes: 95 additions & 0 deletions .github/workflows/unset-environment-new-cli.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# See `unset-environment-old-cli.yml` for reasoning behind the separate tests.
name: PR Check - Test unsetting environment variables for CLI version >= 2.5.1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GO111MODULE: auto
on:
push:
branches:
- main
- releases/v1
- releases/v2
pull_request:
types:
- opened
- synchronize
- reopened
- ready_for_review
workflow_dispatch: {}
jobs:
unset-environment:
strategy:
matrix:
include:
- os: ubuntu-latest
version: stable-20210809
Copy link
Contributor

@aeisenberg aeisenberg Sep 27, 2022

Choose a reason for hiding this comment

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

Not much we can do about this, but we will need to remember to change this when we no longer support CLI v2.5.9.

Copy link
Contributor Author

@angelapwen angelapwen Sep 27, 2022

Choose a reason for hiding this comment

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

Yes, agreed (it's also present in all the other PR checks so we can change them all at once).

- os: ubuntu-latest
version: cached
- os: ubuntu-latest
version: latest
- os: ubuntu-latest
version: nightly-latest
name: Test unsetting environment variables
timeout-minutes: 45
runs-on: ${{ matrix.os }}
steps:
- name: Check out repository
uses: actions/checkout@v3
- name: Prepare test
id: prepare-test
uses: ./.github/prepare-test
with:
version: ${{ matrix.version }}
- uses: ./../action/init
with:
db-location: ${{ runner.temp }}/customDbLocation
tools: ${{ steps.prepare-test.outputs.tools-url }}
env:
TEST_MODE: true
- name: Build code
shell: bash
run: env -i PATH="$PATH" HOME="$HOME" ./build.sh
- uses: ./../action/analyze
id: analysis
env:
TEST_MODE: true
- shell: bash
run: |
CPP_DB="${{ fromJson(steps.analysis.outputs.db-locations).cpp }}"
if [[ ! -d "$CPP_DB" ]] || [[ ! "$CPP_DB" == "${RUNNER_TEMP}/customDbLocation/"* ]]; then
echo "::error::Did not create a database for CPP, or created it in the wrong location." \
"Expected location was '${RUNNER_TEMP}/customDbLocation/*' but actual was '${CPP_DB}'"
exit 1
fi
CSHARP_DB="${{ fromJson(steps.analysis.outputs.db-locations).csharp }}"
if [[ ! -d "$CSHARP_DB" ]] || [[ ! "$CSHARP_DB" == "${RUNNER_TEMP}/customDbLocation/"* ]]; then
echo "::error::Did not create a database for C Sharp, or created it in the wrong location." \
"Expected location was '${RUNNER_TEMP}/customDbLocation/*' but actual was '${CSHARP_DB}'"
exit 1
fi
GO_DB="${{ fromJson(steps.analysis.outputs.db-locations).go }}"
if [[ ! -d "$GO_DB" ]] || [[ ! "$GO_DB" == "${RUNNER_TEMP}/customDbLocation/"* ]]; then
echo "::error::Did not create a database for Go, or created it in the wrong location." \
"Expected location was '${RUNNER_TEMP}/customDbLocation/*' but actual was '${GO_DB}'"
exit 1
fi
JAVA_DB="${{ fromJson(steps.analysis.outputs.db-locations).java }}"
if [[ ! -d "$JAVA_DB" ]] || [[ ! "$JAVA_DB" == "${RUNNER_TEMP}/customDbLocation/"* ]]; then
echo "::error::Did not create a database for Java, or created it in the wrong location." \
"Expected location was '${RUNNER_TEMP}/customDbLocation/*' but actual was '${JAVA_DB}'"
exit 1
fi
JAVASCRIPT_DB="${{ fromJson(steps.analysis.outputs.db-locations).javascript }}"
if [[ ! -d "$JAVASCRIPT_DB" ]] || [[ ! "$JAVASCRIPT_DB" == "${RUNNER_TEMP}/customDbLocation/"* ]]; then
echo "::error::Did not create a database for Javascript, or created it in the wrong location." \
"Expected location was '${RUNNER_TEMP}/customDbLocation/*' but actual was '${JAVASCRIPT_DB}'"
exit 1
fi
PYTHON_DB="${{ fromJson(steps.analysis.outputs.db-locations).python }}"
if [[ ! -d "$PYTHON_DB" ]] || [[ ! "$PYTHON_DB" == "${RUNNER_TEMP}/customDbLocation/"* ]]; then
echo "::error::Did not create a database for Python, or created it in the wrong location." \
"Expected location was '${RUNNER_TEMP}/customDbLocation/*' but actual was '${PYTHON_DB}'"
exit 1
fi
env:
INTERNAL_CODEQL_ACTION_DEBUG_LOC: true
89 changes: 89 additions & 0 deletions .github/workflows/unset-environment-old-cli.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# There was a bug, fixed in CLI v2.5.1, that didn't propagate environment
# variables that the Java tracer needed. Here we test all languages
# except Java for these CLI versions. In `unset-environment-new-cli.yml`
# we test all languages for recent CLI versions.
name: PR Check - Test unsetting environment variables for CLI version < 2.5.1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GO111MODULE: auto
on:
push:
branches:
- main
- releases/v1
- releases/v2
pull_request:
types:
- opened
- synchronize
- reopened
- ready_for_review
workflow_dispatch: {}
jobs:
unset-environment:
strategy:
matrix:
include:
- os: ubuntu-latest
version: stable-20210308
- os: ubuntu-latest
version: stable-20210319
name: Test unsetting environment variables
timeout-minutes: 45
runs-on: ${{ matrix.os }}
steps:
- name: Check out repository
uses: actions/checkout@v3
- name: Prepare test
id: prepare-test
uses: ./.github/prepare-test
with:
version: ${{ matrix.version }}
- uses: ./../action/init
with:
languages: csharp,cpp,go,javascript,python
db-location: ${{ runner.temp }}/customDbLocation
tools: ${{ steps.prepare-test.outputs.tools-url }}
env:
TEST_MODE: true
- name: Build code
shell: bash
run: env -i PATH="$PATH" HOME="$HOME" ./build.sh
- uses: ./../action/analyze
id: analysis
env:
TEST_MODE: true
- shell: bash
run: |
CPP_DB="${{ fromJson(steps.analysis.outputs.db-locations).cpp }}"
if [[ ! -d "$CPP_DB" ]] || [[ ! "$CPP_DB" == "${RUNNER_TEMP}/customDbLocation/"* ]]; then
Copy link
Contributor

@aeisenberg aeisenberg Sep 27, 2022

Choose a reason for hiding this comment

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

So, why exactly do we need to glob here? Is it because we are not sure what the last segement of the directory will be named? And presumably, if there are two entries in the directory, this test will always fail? What if there is a whitespace in this directory segment? I think the test wuld fail, but can this conceivably happen?

Copy link
Contributor Author

@angelapwen angelapwen Sep 27, 2022

Choose a reason for hiding this comment

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

So, why exactly do we need to glob here? Is it because we are not sure what the last segement of the directory will be named?

I'm not exactly sure (I didn't write this test originally) — I think we could remove the * and replace it with the name of the directory, which AFAIK is just the language (cpp for example)

And presumably, if there are two entries in the directory, this test will always fail?

I don't think this is true, because currently the ${RUNNER_TEMP}/customDbLocation/ directory should have all the subdirectories named by each language in them.

What if there is a whitespace in this directory segment? I think the test wuld fail, but can this conceivably happen?

Which directory segment do you mean — the very last one that is just the language? I don't think it can happen, but if it did happen, I think it would fail too. Maybe it would just be better to explicitly write out the language rather than use the * at all.

Copy link
Contributor

@aeisenberg aeisenberg Sep 27, 2022

Choose a reason for hiding this comment

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

Wild...I didn't know that. Just tried it out. if [[ "$var" == * ]]; will match if any file in the current directory matches $var.

It would definitely be clearer if you replaced * with the language name, but I don't think it's a blocker.

echo "::error::Did not create a database for CPP, or created it in the wrong location." \
"Expected location was '${RUNNER_TEMP}/customDbLocation/*' but actual was '${CPP_DB}'"
exit 1
fi
CSHARP_DB="${{ fromJson(steps.analysis.outputs.db-locations).csharp }}"
if [[ ! -d "$CSHARP_DB" ]] || [[ ! "$CSHARP_DB" == "${RUNNER_TEMP}/customDbLocation/"* ]]; then
echo "::error::Did not create a database for C Sharp, or created it in the wrong location." \
"Expected location was '${RUNNER_TEMP}/customDbLocation/*' but actual was '${CSHARP_DB}'"
exit 1
fi
GO_DB="${{ fromJson(steps.analysis.outputs.db-locations).go }}"
if [[ ! -d "$GO_DB" ]] || [[ ! "$GO_DB" == "${RUNNER_TEMP}/customDbLocation/"* ]]; then
echo "::error::Did not create a database for Go, or created it in the wrong location." \
"Expected location was '${RUNNER_TEMP}/customDbLocation/*' but actual was '${GO_DB}'"
exit 1
fi
JAVASCRIPT_DB="${{ fromJson(steps.analysis.outputs.db-locations).javascript }}"
if [[ ! -d "$JAVASCRIPT_DB" ]] || [[ ! "$JAVASCRIPT_DB" == "${RUNNER_TEMP}/customDbLocation/"* ]]; then
Copy link
Contributor

@aeisenberg aeisenberg Sep 27, 2022

Choose a reason for hiding this comment

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

Is there a reason why * is outside of the quotes? I don't think it changes the meaning. Also, does $JAVASCRIPT_DB end in *? I guess so, or else the workflow would be failing.

Copy link
Contributor Author

@angelapwen angelapwen Sep 27, 2022

Choose a reason for hiding this comment

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

Yes, it's a wildcard character when outside quotes. @adityasharad and I debugged the reason it failed just now and it was because * was in the quotes and not considered a wildcard 😄

Copy link
Contributor

@adityasharad adityasharad Sep 27, 2022

Choose a reason for hiding this comment

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

Just looked at this one together -- you need the * outside the quotes so that Bash glob-expands it, otherwise it gets treated as a literal * character.

Copy link
Contributor

@aeisenberg aeisenberg Sep 27, 2022

Choose a reason for hiding this comment

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

Bash is awesome. :) Thanks for explaining.

echo "::error::Did not create a database for Javascript, or created it in the wrong location." \
"Expected location was '${RUNNER_TEMP}/customDbLocation/*' but actual was '${JAVASCRIPT_DB}'"
exit 1
fi
PYTHON_DB="${{ fromJson(steps.analysis.outputs.db-locations).python }}"
if [[ ! -d "$PYTHON_DB" ]] || [[ ! "$PYTHON_DB" == "${RUNNER_TEMP}/customDbLocation/"* ]]; then
echo "::error::Did not create a database for Python, or created it in the wrong location." \
"Expected location was '${RUNNER_TEMP}/customDbLocation/*' but actual was '${PYTHON_DB}'"
exit 1
fi
env:
INTERNAL_CODEQL_ACTION_DEBUG_LOC: true
49 changes: 0 additions & 49 deletions pr-checks/checks/unset-environment.yml
View file
Open in desktop

This file was deleted.