Skip to content

Commit

Permalink
chore: add QA checks to Test workflow (#219)
Browse files Browse the repository at this point in the history
This change adds a new `QA` job to the existing `Test` workflow. It is
done in a way that the `Test rollup` job can continue to be an enforced
status check, but with the addition of this job. A new optional `tox`
test environment named `qa` was added and is called by the `QA` job. The
environment can also be called by users locally. It relies on simply
running `pre-commit` on the existing hooks defined in the repository:

* trim trailing whitespace
* fix end of files
* check yaml
* check for added large files
* `black`
* `pyupgrade`
* analyze lockfile with `phylum-ci`
  * This one is skipped

The `phylum-ci` pre-commit hook is skipped in the `QA` job since:

* The current GitHub integration expects to *only* run in a PR context
  * The `Test` workflow also includes `workflow_dispatch` and `push`
  * The integration will fail even if a `GITHUB_TOKEN` is provided
* The `phylum-ci` action will already be run for pull request triggers

It is possible this restriction can be lifted in the future if the
GitHub integration is updated to account for additional execution
contexts beyond pull requests.

The `pre-commit` package was added as a dependency in the `qa` group and
can therefore be kept under configuration control and used by `poetry`.
Documentation was updated to make all the new usage patterns explicit.

Additional QA checks and `pre-commit` hooks will be added separately.

Additional changes made include:

* Fix a missing version check normalization
  * When no CLI version is specified and no CLI is installed
  * The version was reported as `latest`
* Version is now current latest in "vMajor.Minor.Bugfix" format

Closes #14
  • Loading branch information
maxrake authored Apr 5, 2023
1 parent 8325cc3 commit 6545116
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 29 deletions.
61 changes: 47 additions & 14 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,55 @@ on:
branches:
- main

defaults:
run:
shell: bash

jobs:
QA:
name: Quality Assurance
runs-on: ubuntu-latest
strategy:
matrix:
# It's only one Python version specified in a "matrix", but on purpose to stay DRY
python-version: ["3.11"]
steps:
- name: Checkout the repo
uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0

- name: Install poetry
run: pipx install poetry

- name: Configure poetry
run: poetry config virtualenvs.in-project true

- name: Set up Python
uses: actions/setup-python@d27e3f3d7c64b4bbf8e4abfb9b63b83e846e0435 # v4.5.0
with:
python-version: ${{ matrix.python-version }}
cache: 'poetry'

- name: Install the project with poetry
run: |
poetry env use python${{ matrix.python-version }}
poetry lock --check
poetry install --verbose --no-root --sync --with qa
- name: Run tox via poetry
env:
# Skip the `phylum-ci` pre-commit hook since:
# * The current GitHub integration expects to *only* be run in a PR context
# * The `phylum-ci` action will already be run for pull request triggers
SKIP: phylum-ci
run: poetry run tox run -e qa

test-matrix:
name: Test on Python ${{ matrix.python-version }}
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
python-version: ["3.8", "3.9", "3.10", "3.11"]
defaults:
run:
shell: bash
steps:
- name: Checkout the repo
uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0
Expand Down Expand Up @@ -55,9 +93,6 @@ jobs:
matrix:
# It's only one Python version specified in a "matrix", but on purpose to stay DRY
python-version: ["3.11"]
defaults:
run:
shell: bash
env:
DOCKER_BUILDKIT: 1
steps:
Expand Down Expand Up @@ -123,18 +158,16 @@ jobs:
# the repo settings without needing to update those settings everytime the test jobs are updated.
test-rollup:
name: Test rollup
if: always()
needs: [test-matrix, docker]
runs-on: ubuntu-latest
if: always()
needs: [QA, test-matrix, docker]
steps:
- name: Check for test jobs failure
if: (needs.test-matrix.result != 'success') || (needs.docker.result != 'success')
shell: bash
if: (needs.QA.result != 'success') || (needs.test-matrix.result != 'success') || (needs.docker.result != 'success')
run: |
echo "One or more test matrix jobs was/were not successful"
echo "At least one test job was not successful"
exit 1
- name: Confirm test jobs success
if: (needs.test-matrix.result == 'success') && (needs.docker.result == 'success')
shell: bash
run: echo "All test matrix jobs were successful"
if: (needs.QA.result == 'success') && (needs.test-matrix.result == 'success') && (needs.docker.result == 'success')
run: echo "All test jobs were successful"
26 changes: 14 additions & 12 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,10 @@ Here's how to set up `phylum-ci` for local development.
# Install the main dependencies only:
poetry install --sync
# Alternatively, specific dependency groups can be installed at the same time.
# It makes sense to add the "test" group now if new code is going to be added and tested:
poetry install --sync --with test
# Alternatively, specific dependency groups can be installed at the
# same time. It makes sense to add the "test" and "qa" groups now
# if new code is going to be added and tested:
poetry install --sync --with test,qa
```

6. Create a branch for local development:
Expand Down Expand Up @@ -180,12 +181,13 @@ Here's how to set up `phylum-ci` for local development.
phylum analyze poetry.lock
```

8. When you're done making changes, check that your changes pass the tests:
8. When you're done making changes, check that your changes pass QA and the tests:
```sh
# Ensure the "test" dependency group is installed, if not done previously
poetry install --sync --with test
poetry run tox
# Ensure the "test" and "qa" dependency groups are installed, if not done previously
poetry install --sync --with test,qa
poetry run tox run -e qa
poetry run tox run-parallel
```
9. Commit your changes and push your branch to GitHub:
Expand Down Expand Up @@ -232,16 +234,16 @@ interact with `pytest` by passing additional positional arguments:
```sh
# passing additional options to pytest requires using the double dash
# escape twice, once for escaping `poetry` and again for escaping `tox`
poetry run -- tox -e py310 -- --help
poetry run -- tox run -e py310 -- --help
# run a specific test module across all test environments
poetry run -- tox -- tests/unit/test_package_metadata.py
# run a specific test module across all test environments in parallel
poetry run -- tox run-parallel -- tests/unit/test_package_metadata.py
# run a specific test module across a specific test environment
poetry run -- tox -e py39 -- tests/unit/test_package_metadata.py
poetry run -- tox run -e py39 -- tests/unit/test_package_metadata.py
# run a specific test function within a test module, in a specific test environment
poetry run -- tox -e py310 -- tests/unit/test_package_metadata.py::test_python_version
poetry run -- tox run -e py310 -- tests/unit/test_package_metadata.py::test_python_version
```
To run a script entry point with the local checkout of the code (in develop mode), use `poetry`:
Expand Down
80 changes: 79 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ optional = true

[tool.poetry.group.qa.dependencies]
types-requests = "*"
pre-commit = "*"
tox = "*"

[tool.black]
line-length = 120
Expand Down
2 changes: 1 addition & 1 deletion src/phylum/ci/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ def main(args: Optional[Sequence[str]] = None) -> int:
parsed_args.version = version_check(parsed_args.version)
else:
print(" [+] Phylum CLI version not specified")
parsed_args.version = default_phylum_cli_version()
parsed_args.version = version_check(default_phylum_cli_version())
print(f" [*] Using Phylum CLI version: {parsed_args.version}")

# Detect which CI environment, if any, we are in
Expand Down
2 changes: 1 addition & 1 deletion src/phylum/ci/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

SUCCESS_DETAILS = "The Phylum risk analysis is complete and did not identify any issues."

# Expandable HTML providing information on why there was a failure
# Background text providing information on why there was a failure
FAILURE_DETAILS = """
This repository analyzes risk of new dependencies with Phylum. An administrator
of this repository has set score requirements for Phylum's five risk domains.
Expand Down
14 changes: 14 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
[tox]
# Sub-commands were introduced in v4. These are used in workflows and docs.
min_version = 4
envlist = py38, py39, py310, py311
isolated_build = true

Expand All @@ -10,10 +12,22 @@ python =
3.11: py311

[testenv]
description = Test environment for minor Python version
passenv = *
# Skip the local package install so that `poetry` can handle installing
# all of the dependencies and do so only from the `poetry.lock` lockfile.
skip_install = true
allowlist_externals = poetry
commands =
poetry lock --check
poetry install --verbose --sync --with test
poetry run python -m pip list
poetry run pytest {posargs}

[testenv:qa]
description = Quality Assurance (QA) checks
commands =
poetry lock --check
poetry install --verbose --sync --with qa
poetry run python -m pip list
poetry run pre-commit run --all-files --verbose

0 comments on commit 6545116

Please sign in to comment.