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 mast remote tests that need boto3 if it is not installed #2755

Merged
merged 1 commit into from
Jun 22, 2023

Conversation

eerovaher
Copy link
Member

If astroquery is installed with pip install astroquery[test] then pytest --remote-data=any -k test_get_cloud_uri astroquery/mast results in two tests failing with ModuleNotFoundError: No module named 'boto3' because although boto3 is listed among the all dependencies, it is not listed among the test dependencies:

astroquery/setup.cfg

Lines 136 to 140 in aa9ce56

all=
mocpy>=0.9
astropy-healpix
boto3
regions

astroquery/setup.cfg

Lines 127 to 131 in aa9ce56

test=
pytest-astropy
matplotlib
pytest-dependency
pytest-rerunfailures

The tests therefore cannot assume that boto3 is installed. There are two simple solutions to this problem. One would be to add boto3 to the test dependencies, but adding a whole new dependency only for the two remote tests in mast is in my opinion excessive. This pull request implements the other simple option, which is to check whether boto3 is installed in the two tests that require it and to skip them if it isn't.

Two `mast` remote tests require `boto3`, which is not installed when
installing `astroquery` with the test dependencies. It therefore cannot
be assumed that `boto3` is installed when running the tests. If it isn't
installed then it is better to skip the tests in question than to have
them fail with `ModuleNotFoundError`.
@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

Merging #2755 (890f617) into main (aa9ce56) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2755   +/-   ##
=======================================
  Coverage   66.04%   66.04%           
=======================================
  Files         233      233           
  Lines       17927    17927           
=======================================
  Hits        11840    11840           
  Misses       6087     6087           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@bsipocz bsipocz left a comment

Choose a reason for hiding this comment

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

I'm OK with this, though don't think adding boto3 to test dependencies would be such an issue, after all we pull in mpl for the sake of a few doctests.

@bsipocz bsipocz added this to the v0.4.7 milestone Jun 22, 2023
@bsipocz bsipocz merged commit 227ded6 into astropy:main Jun 22, 2023
@eerovaher eerovaher deleted the mast-tests-boto3 branch June 22, 2023 22:11
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