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

Offline testing and test data fetching improvements #1473

Merged
merged 24 commits into from
Sep 12, 2023
Merged

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Sep 8, 2023

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • CHANGES.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

  • Adds pytest-socket to the testing dependencies, in order to block HTTP requests
  • Adds a new pytest marker "requires_internet" for tests that absolutely depend on internet access.
  • Fixed an error in the pytest configuration that impeded copying of testing data to thread safe caches of workers (this should always occur).
  • Added handling for pytest-socket's SocketBlockedError when attempting to fetch md5 validation files for cached testing data.
  • Added a tox modifier for running tests without internet access (tox -e offline)
    • Option to call tox -e offline-prefetch to run the data prefetching command before sockets are disabled in pytest call.
  • Added a build using Python3.10 to the GitHub Workflows that uses the new offline modifier in the tox configuration.
  • For better accuracy, the tox -e black recipe has been renamed to tox -e lint, as this configuration already included several other linting checks.
  • Added a CLI command ($ xclim prefetch_testing_data) for manually calling the populate_testing_data() command with branch (--branch/-b) options.
  • Updated the testing data used in the analogs.ipynb notebook to use the testing data now found in Ouranosinc/xclim-testdata's main branch (see: Add analogs notebook data to main branch xclim-testdata#26)

xclim now has a dedicated CLI command for prefetching the Ouranosinc/xclim-testdata:

$ xclim prefetch_testing_data
# or, for a specific testdata branch
$ xclim prefetch_testing_data --branch my_testdata_branch

Running pytest will still perform the fetching of data on first run if the user does not have an xclim-testdata folder in their platform-dependent cache location (/home/$USER/.cache/xclim-testdata on Linux).

Does this PR introduce a breaking change?

Yes:

  • tox is now configured to pass positional arguments to the pytest calls directly. This gives developers much more control when selecting/deselecting pytest markers. To replicate previous behaviour (skipping slow tests), most tox invocations should resemble the following:
$ tox -e pyXYZ -- -m "not slow"
  • xclim development now requires the pytest-socket plugin (has been explicitly added to the development requirements). This plugin allows developers to mimic being offline when testing new xclim features or bug fixes, if they so choose.

  • For better accuracy, the tox -e black recipe has been renamed to tox -e lint, as this configuration already included several other linting checks.

Other information:

https://github.com/miketheman/pytest-socket
https://tox.wiki/en/latest/config.html#substitutions-for-positional-arguments-in-commands

@Zeitsperre Zeitsperre added the information For development/intsructional purposes label Sep 8, 2023
@Zeitsperre Zeitsperre added this to the v0.46.0 milestone Sep 8, 2023
@Zeitsperre Zeitsperre self-assigned this Sep 8, 2023
@github-actions github-actions bot added the CI Automation and Contiunous Integration label Sep 8, 2023
@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Note
It appears that this Pull Request modifies the main.yml workflow.

On inspection, the XCLIM_TESTDATA_BRANCH environment variable is set to the most recent tag (v2023.9.12).

No further action is required.

@Zeitsperre Zeitsperre added the approved Approved for additional tests label Sep 11, 2023
… `tox` and GitHub Workflows is now its own separate build configuration, `tox` config now offers the `test` label for easier testing, `black` configuration in `tox` and GitHub Workflows is now called `lint` since it contains much more than `black` checks.
…pdated pytest and tox usage to reflect new conventions
@github-actions github-actions bot added the docs Improvements to documenation label Sep 11, 2023
@Zeitsperre Zeitsperre marked this pull request as ready for review September 11, 2023 17:37
CONTRIBUTING.rst Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
…will now bypass `--disable-socket` for testing purposes, added more missing prefetch files, update `test_get_failure` expectations, `gather_session_data` now has a much clearer explanation docstring
…ction-scoped, mark `test_reduce_dims` as `requires_internet` as it relies on remote xarray tutorial data
…ptionally prefetch data (mainly for GitHub CI purposes), update contributing documentation to reflect changes
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added the API Interfacing and User Concerns label Sep 12, 2023
@Zeitsperre Zeitsperre changed the title Offline testing Offline testing and test data fetching improvements Sep 12, 2023
@Zeitsperre Zeitsperre requested a review from huard September 12, 2023 19:38
Copy link
Collaborator

@huard huard left a comment

Choose a reason for hiding this comment

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

LGTM,
thanks

@coveralls
Copy link

coveralls commented Sep 12, 2023

Coverage Status

coverage: 90.468% (-0.2%) from 90.674% when pulling d80ce92 on offline-testing into 44079ac on master.

@Zeitsperre Zeitsperre merged commit 66cf19c into master Sep 12, 2023
13 checks passed
@Zeitsperre Zeitsperre deleted the offline-testing branch September 12, 2023 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Interfacing and User Concerns approved Approved for additional tests CI Automation and Contiunous Integration docs Improvements to documenation information For development/intsructional purposes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test suite does not run without internet access
3 participants