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

Use nox for our GitHub Actions workflows #872

Merged
merged 11 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
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
24 changes: 0 additions & 24 deletions .github/actions/install-pkg/action.yml

This file was deleted.

13 changes: 9 additions & 4 deletions .github/workflows/integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,21 @@ jobs:
# this point.
ref: ${{ github.event.pull_request.head.sha }}

- name: Install package with dependencies
uses: ./.github/actions/install-pkg
- name: Install uv
uses: astral-sh/setup-uv@v3.2.2
with:
python-version: ${{ matrix.python-version }}
enable-cache: true

- name: Setup nox
uses: wntrblm/nox@2024.10.09
with:
python-versions: ${{ matrix.python-version }}

- name: Run integration tests
env:
EARTHDATA_USERNAME: ${{ secrets.EDL_USERNAME }}
EARTHDATA_PASSWORD: ${{ secrets.EDL_PASSWORD }}
run: ./scripts/integration-test.sh
run: nox -s integration-tests -- --cov=earthaccess --cov-report=term-missing --capture=no --tb=native --log-cli-level=INFO
chuckwondo marked this conversation as resolved.
Show resolved Hide resolved

- name: Upload coverage report
# Don't upload coverage when using the `act` tool to run the workflow locally
Expand Down
17 changes: 5 additions & 12 deletions .github/workflows/test-mindeps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,17 @@ jobs:
uses: actions/checkout@v4.1.1

- name: Install uv
uses: astral-sh/setup-uv@v3
uses: astral-sh/setup-uv@v3.2.2
with:
version: "0.4.7"
enable-cache: true

- name: Set up Python
uses: actions/setup-python@v5
- name: Setup nox
uses: wntrblm/nox@2024.10.09
with:
python-version: 3.9

- name: Install minimum-compatible dependencies
run: uv sync --resolution lowest-direct --extra test

- name: Install earthaccess
run: uv pip install --no-deps .
python-versions: 3.9

- name: Test
run: uv run pytest tests/unit --cov=earthaccess --cov=tests --cov-report=term-missing --capture=no --tb=native --log-cli-level=INFO
run: nox -s test-min-deps -- --verbose --cov=earthaccess --cov-report=term-missing --capture=no --tb=native --log-cli-level=INFO

- name: Upload coverage
# Don't upload coverage when using the `act` tool to run the workflow locally
Expand Down
14 changes: 10 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,21 @@ jobs:
- name: Checkout sources
uses: actions/checkout@v4

- uses: ./.github/actions/install-pkg
- name: Install uv
uses: astral-sh/setup-uv@v3.2.2
with:
python-version: ${{ matrix.python-version }}
enable-cache: true

- name: Setup nox
uses: wntrblm/nox@2024.10.09
with:
python-versions: ${{ matrix.python-version }}

- name: Typecheck
run: mypy
run: nox -s typecheck

- name: Test
run: pytest tests/unit --verbose --cov=earthaccess --cov-report=term-missing --capture=no --tb=native --log-cli-level=INFO
run: nox -s tests -- --verbose --cov=earthaccess --cov-report=term-missing --capture=no --tb=native --log-cli-level=INFO

- name: Upload coverage
# Don't upload coverage when using the `act` tool to run the workflow locally
Expand Down
17 changes: 9 additions & 8 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html)

## [Unreleased]

- Fix `earthaccess.download` to not ignore errors by default
([#581](https://github.com/nsidc/earthaccess/issues/581))
([**@Sherwin-14**](https://github.com/Sherwin-14),
[**@chuckwondo**](https://github.com/chuckwondo),
[**@mfisher87**](https://github.com/mfisher87))

### Changed

- Use built-in `assert` statements instead of `unittest` assertions in
Expand All @@ -25,16 +19,23 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html)
location ([#480](https://github.com/nsidc/earthaccess/issues/480))
([@chuckwondo](https://github.com/chuckwondo))
- Add `nox` session for running integration tests locally
([#815](https://github.com/nsidc/earthaccess/issues/815)) ([@chuckwondo](https://github.com/chuckwondo))
([#815](https://github.com/nsidc/earthaccess/issues/815); [@chuckwondo](https://github.com/chuckwondo) and ([#872](https://github.com/nsidc/earthaccess/issues/872); [@jhkennedy](https://github.com/jhkennedy))
- Auto-add comment to PR that requires maintainer to review and re-run
integration tests ([#824](https://github.com/nsidc/earthaccess/issues/824))
([@chuckwondo](https://github.com/chuckwondo))

### Removed

### Fixed
- The `scripts/integration-test.sh` script has been removed in favor of the `integration-tests` nox session.
([#872](https://github.com/nsidc/earthaccess/issues/872)) ([@jhkennedy](https://github.com/jhkennedy))

### Fixed

- `earthaccess.download` will not ignore errors by default
([#581](https://github.com/nsidc/earthaccess/issues/581))
([**@Sherwin-14**](https://github.com/Sherwin-14),
[**@chuckwondo**](https://github.com/chuckwondo),
[**@mfisher87**](https://github.com/mfisher87))
- Integration tests no longer clobber existing `.netrc` file
([#806](https://github.com/nsidc/earthaccess/issues/806))
(@chuckwondo)
Expand Down
49 changes: 33 additions & 16 deletions docs/contributing/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,28 +20,45 @@ If you don't have pipx (pip for applications), then you can install with
pip is reasonable). If you use macOS, then pipx and nox are both in brew, use
`brew install pipx nox`.

To use, run `nox` without any arguments. This will run type checks and unit
tests using the installed version of Python on your system.
To use, run `nox` without any arguments. This will run the type check and unit
test "sessions" (tasks) using your local (and active) Python version.
Nox handles everything for you, including setting up a temporary virtual
environment for each run.

You can see all available sessions with `nox --list`:

```
$ nox --list
Sessions defined in earthaccess/noxfile.py:

* typecheck -> Typecheck with mypy.
* tests -> Run the unit tests.
- test-min-deps -> Run the unit tests using the lowest compatible version of all direct dependencies.
- integration-tests -> Run the integration tests.
- build-pkg -> Build a source distribution and binary distribution (wheel).
- serve-docs -> Build the documentation and serve it.

sessions marked with * are selected, sessions marked with - are skipped.
```
jhkennedy marked this conversation as resolved.
Show resolved Hide resolved
You can also run individual tasks (_sessions_ in `nox` parlance, hence the `-s`
option below), like so:

```console
nox -s typecheck # Run typechecks
nox -s tests # Run unit tests
nox -s integration-tests # Run integration tests (see note below)
nox -s serve_docs # Build and serve the docs
nox -s build_pkg # Build an SDist and Wheel
```bash
nox -s integration-tests
```
and pass options to the underlying session like:
```bash
nox -s integration-tests -- [ARGS]
jhkennedy marked this conversation as resolved.
Show resolved Hide resolved
```

Nox handles everything for you, including setting up a temporary virtual
environment for each run.
!!! tip

In order to run integration tests locally, you must set the
environment variables `EARTHDATA_USERNAME` and `EARTHDATA_PASSWORD` to your
username and password, respectively, of your
[NASA Earthdata](https://urs.earthdata.nasa.gov/) account (registration is
free).


**NOTE:** In order to run integration tests locally, you must set the
environment variables `EARTHDATA_USERNAME` and `EARTHDATA_PASSWORD` to your
username and password, respectively, of your
[NASA Earthdata](https://urs.earthdata.nasa.gov/) account (registration is
free).

## Manual development environment setup

Expand Down
26 changes: 22 additions & 4 deletions noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,19 @@ def tests(session: nox.Session) -> None:
session.run(
"pytest",
"tests/unit",
"-rxXs", # Show provided reason in summary for (x)fail, (X)pass, and (s)kipped statuses
"-rxXs", # Show provided reason in summary for (x)fail, (X)pass, and (s)kipped tests
*session.posargs,
)


@nox.session(name="test-min-deps", python="3.9", venv_backend="uv")
def test_min_deps(session: nox.Session) -> None:
"""Run the unit tests using the lowest compatible version of all direct dependencies."""
session.install("--resolution", "lowest-direct", "--editable", ".[test]")
session.run(
"pytest",
"tests/unit",
"-rxXs", # Show provided reason in summary for (x)fail, (X)pass, and (s)kipped tests
*session.posargs,
)

Expand All @@ -37,17 +49,23 @@ def integration_tests(session: nox.Session) -> None:
"""Run the integration tests."""
session.install("--editable", ".[test]")
session.run(
"scripts/integration-test.sh",
"pytest",
"tests/integration",
"-rxXs", # Show provided reason in summary for (x)fail, (X)pass, and (s)kipped tests
*session.posargs,
env=dict(
EARTHDATA_USERNAME=os.environ["EARTHDATA_USERNAME"],
EARTHDATA_PASSWORD=os.environ["EARTHDATA_PASSWORD"],
),
external=True,
# NOTE: integration test are permitted to pass if the failure rate was less than a hardcoded threshold.
# PyTest will return 99 if there were some failures, but less than the threshold. For more details, see:
# `pytest_sessionfinish` in tests/integration/conftest.py
success_codes=[0, 99],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we also need to replace the hard-coded 99 with the value of EARTHACCESS_ALLOWABLE_FAILURE_STATUS_CODE here too (defaulting to 99, if not set)?

Copy link
Collaborator Author

@jhkennedy jhkennedy Nov 7, 2024

Choose a reason for hiding this comment

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

I don't think so here because we're not passing that environment variable down to the test environment, so it should always use the default 99.:
https://github.com/nsidc/earthaccess/pull/872/files#diff-f7a16a65f061822bcc73b8296f4dc837353d379d8d9cc5307982cb6941442835R56-R59

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess that's why I'm a bit confused. Where/when/how do you expect to set the var and have it used? You mentioned something about running tests from your IDE, so I guess I'm wondering how that would work (I just run them from the terminal, and haven't attempted running from my IDE).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it's only useful for directly invoking pytest tests/integration ... either in the terminal or via your IDE so that you get a successful status code. If you invoke via nox, it handles that bit for you, but making VS Code and PyCharm aware of nox is significantly harder than making them aware of pytest.

Preview of the upcoming PR where I go into it a bit in the context of an IDE:
https://github.com/nsidc/earthaccess/blob/dev-guide/docs/contributing/development.md#ide-setup

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm going to merge this one and open the next one so we can continue the conversation there.

(ideally, we wouldn't have flakey tests...)

)


@nox.session
@nox.session(name="build-pkg")
def build_pkg(session: nox.Session) -> None:
"""Build a source distribution and binary distribution (wheel)."""
build_path = DIR.joinpath("build")
Expand All @@ -58,7 +76,7 @@ def build_pkg(session: nox.Session) -> None:
session.run("python", "-m", "build")


@nox.session
@nox.session(name="serve-docs")
def serve_docs(session: nox.Session) -> None:
"""Build the documentation and serve it."""
session.install("--editable", ".[docs]")
Expand Down
16 changes: 0 additions & 16 deletions scripts/integration-test.sh

This file was deleted.

12 changes: 11 additions & 1 deletion tests/integration/conftest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import pathlib
from warnings import warn

import earthaccess
import pytest
Expand All @@ -22,6 +23,9 @@ def pytest_sessionfinish(session, exitstatus):
ratio will change depending on which tests are executed! E.g. executing integration
tests and unit tests at the same time allows more tests to fail than executing
integration tests alone.

NOTE: The return exit code can be customized with the `EARTHACCESS_ALLOWABLE_FAILURE_STATUS_CODE`
environment variable.
"""
if exitstatus != pytest.ExitCode.TESTS_FAILED:
# Exit status 1 in PyTest indicates "Tests were collected and run but some of
Expand All @@ -32,7 +36,13 @@ def pytest_sessionfinish(session, exitstatus):

failure_rate = (100.0 * session.testsfailed) / session.testscollected
if failure_rate <= ACCEPTABLE_FAILURE_RATE:
session.exitstatus = 99
status_code = os.environ.get("EARTHACCESS_ALLOWABLE_FAILURE_STATUS_CODE", 99)
warn(
f"\nWARNING: The integration test suite has returned {status_code} because the "
"failure rate was less than a hardcoded threshold. For more details see:\n"
"`pytest_sessionfinish` in tests/integration/conftest.py."
)
session.exitstatus = status_code


@pytest.fixture
Expand Down
Loading