Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
# Warning: This file is generated automatically, and should not be modified.
# Instead, please modify the template in the pr-checks directory and run:
# pip install ruamel.yaml && python3 sync.py
# to regenerate this file.

name: PR Check - Test unsetting environment variables
name: PR Check - Test unsetting environment variables for CLI version > 2.5
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GO111MODULE: auto
Expand All @@ -25,10 +20,6 @@ jobs:
strategy:
matrix:
include:
- os: ubuntu-latest
version: stable-20210308
- os: ubuntu-latest
version: stable-20210319
- os: ubuntu-latest
version: stable-20210809

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

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
Expand Down
80 changes: 80 additions & 0 deletions .github/workflows/unset-environment-old-cli.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
name: PR Check - Test unsetting environment variables for CLI version < 2.5
Comment thread
angelapwen marked this conversation as resolved.
Outdated
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 }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
CPP_DB=${{ fromJson(steps.analysis.outputs.db-locations).cpp }}
CPP_DB="${{ fromJson(steps.analysis.outputs.db-locations).cpp }}"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Have done this!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm curious — why does CPP_DB not need the quotes here, but you've added them to its usage later on? Is it because it shouldn't be added while it's being assigned to?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When you are defining a bash variable, you don't need quotes around its name, only its value. When you are using a bash variable, it should always be quoted.

If you don't quote at the appropriate times, whitespace will be considered separate arguments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here's an example. Let's say ${{ fromJson(steps.analysis.outputs.db-locations).cpp }} resolves to /path/with a space.

This command:

CPP_DB=${{ fromJson(steps.analysis.outputs.db-locations).cpp }}

will be interpreted as setting the variable CPP_DB/path/with and apply this to a command a with an argument space.

CPP_DB="${{ fromJson(steps.analysis.outputs.db-locations).cpp }}"

Will simply set the CPP_DB variable to /path/with a space. Bash is arcane and learning all these rules takes a long time.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the example, that helps! I didn't realize that. Makes sense as to why the variable doesn't need quotes when it's being defined, then.

if [[ ! -d $CPP_DB ]] || [[ ! $CPP_DB == ${{ runner.temp }}/customDbLocation/* ]]; then
Comment thread
angelapwen marked this conversation as resolved.
Outdated
echo "Did not create a database for CPP, or created it in the wrong location."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
echo "Did not create a database for CPP, or created it in the wrong location."
echo "Did not create a database for CPP, or created it in the wrong location. Expected it in $CPP_DB."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I actually read the test differently — I thought expected would be at ${{ runner.temp }}/customDbLocation/* and actual would be $CPP_DB.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah...apologies. You might be right here.

exit 1
fi
CSHARP_DB=${{ fromJson(steps.analysis.outputs.db-locations).csharp }}
if [[ ! -d $CSHARP_DB ]] || [[ ! $CSHARP_DB == ${{ runner.temp }}/customDbLocation/* ]]; then
echo "Did not create a database for C Sharp, or created it in the wrong location."
exit 1
fi
GO_DB=${{ fromJson(steps.analysis.outputs.db-locations).go }}
if [[ ! -d $GO_DB ]] || [[ ! $GO_DB == ${{ runner.temp }}/customDbLocation/* ]]; then
echo "Did not create a database for Go, or created it in the wrong location."
exit 1
fi
JAVASCRIPT_DB=${{ fromJson(steps.analysis.outputs.db-locations).javascript }}
if [[ ! -d $JAVASCRIPT_DB ]] || [[ ! $JAVASCRIPT_DB == ${{ runner.temp }}/customDbLocation/* ]]; then
echo "Did not create a database for Javascript, or created it in the wrong location."
exit 1
fi
PYTHON_DB=${{ fromJson(steps.analysis.outputs.db-locations).python }}
if [[ ! -d $PYTHON_DB ]] || [[ ! $PYTHON_DB == ${{ runner.temp }}/customDbLocation/* ]]; then
echo "Did not create a database for Python, or created it in the wrong location."
exit 1
fi
env:
INTERNAL_CODEQL_ACTION_DEBUG_LOC: true
49 changes: 0 additions & 49 deletions pr-checks/checks/unset-environment.yml

This file was deleted.