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

Skip webbpsf tests by default #958

Merged

Conversation

WilliamJamieson
Copy link
Collaborator

Currently the romancal testing runs tests which require the webbpsf's input data by default. This is undesirable because it requires users to have to set up this data in order for the test suite to complete successfully.

This PR adds the --webbpsf marker which enables these tests. This means that the --webbpsf flag must be passed to pytest in order to run the tests in question. This means that by default the webbpsf marked tests are not run. However,
this PR does add this marker to every CI job both on GitHub and Jenkins, so that the CI will always run the webbpsf tests. This means we will know prior to merging any changes if these tests are broken while also removing the burden of requiring outside users to not have to worry about webbpsf.

Checklist

  • added entry in CHANGES.rst under the corresponding subsection
  • updated relevant tests
  • updated relevant documentation
  • updated relevant milestone(s)
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below. How to run regression tests on a PR

@WilliamJamieson WilliamJamieson requested a review from a team as a code owner October 27, 2023 15:46
@WilliamJamieson WilliamJamieson requested review from zacharyburnett and bmorris3 and removed request for a team October 27, 2023 15:47

def pytest_addoption(parser):
parser.addoption(
"--webbpsf", action="store_true", default=False, help="run webbpsf tests"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically, we could set default to be true if users happen to have the WEBBPSF_PATH environment variable set.

Copy link
Collaborator

@zacharyburnett zacharyburnett left a comment

Choose a reason for hiding this comment

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

Looks good! Should the readme instructions also be updated?

Additionally, currently WebbPSF data is also required. Follow [these instructions to download the data files / point to existing files on the shared internal network](https://webbpsf.readthedocs.io/en/latest/installation.html#data-install).

-  Additionally, currently WebbPSF data is also required. Follow [these instructions to download the data files / point to existing files on the shared internal network](https://webbpsf.readthedocs.io/en/latest/installation.html#data-install). 
+  Additionally, if it is desired to run tests against WebbPSF data, use the `pytest --webbpsf` flag or the `-webbpsf` tox factor and follow [these instructions to download the data files / point to existing files on the shared internal network](https://webbpsf.readthedocs.io/en/latest/installation.html#data-install). 

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 27, 2023
@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Files Coverage Δ
conftest.py 66.66% <66.66%> (ø)

📢 Thoughts on this report? Let us know!.

@zacharyburnett zacharyburnett merged commit 78bc549 into spacetelescope:main Oct 27, 2023
27 of 29 checks passed
@WilliamJamieson WilliamJamieson deleted the testing/skip_webbpsf branch October 27, 2023 17:38
ddavis-stsci pushed a commit to ddavis-stsci/romancal that referenced this pull request Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants