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

Remove astroquery/utils/testing_tools.py #2287

Merged
merged 3 commits into from Feb 7, 2022

Conversation

eerovaher
Copy link
Member

The deleted file defined two pytest fixtures. Neither was used anywhere, and their functionality is provided by pytest-remotedata.

Closes #1134

Copy link
Contributor

@keflavich keflavich left a comment

Choose a reason for hiding this comment

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

Thanks. These were obsoleted by this functionality being incorporated somewhere into the astropy testing framework, though I don't remember where or when

@keflavich
Copy link
Contributor

(my approval is contingent on tests passing)


# Import MockResponse to keep the API while it's temporarily factored out to
# a separate file to avoid requiring pytest as a dependency in non-test code
from .mocks import MockResponse
Copy link
Member

Choose a reason for hiding this comment

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

I'm OK with the removal of this file, if you also clean up the mock import in the code.

Copy link
Member Author

Choose a reason for hiding this comment

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

I ran the tests locally with

python setup.py test

and nothing failed. Running the tests with tox does lead to the same failures as were seen in CI.

Copy link
Member

Choose a reason for hiding this comment

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

I think some temporary files may have been left behind when you tested locally, but not when it was a fully independent build for tox. I expect that the testing would have failed after a git clean.

Copy link
Member Author

Choose a reason for hiding this comment

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

I expect that the testing would have failed after a git clean.

Indeed they do.

@bsipocz
Copy link
Member

bsipocz commented Feb 7, 2022

Also a short changelog along the lines of "removal of obsolete testing tools" would be useful, added to the last changelog section.

The `MockResponse` class utilised in many tests is defined in
`astroquery.utils.mocks`, but many modules imported it via
`astroquery.utils.testing_tools`.
The deleted file defined two pytest fixtures. Neither was used anywhere,
and their functionality is provided by `pytest-remotedata`.
@codecov
Copy link

codecov bot commented Feb 7, 2022

Codecov Report

Merging #2287 (0bd6724) into main (27515e4) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2287      +/-   ##
==========================================
+ Coverage   62.72%   62.75%   +0.02%     
==========================================
  Files         131      130       -1     
  Lines       16854    16835      -19     
==========================================
- Hits        10572    10564       -8     
+ Misses       6282     6271      -11     
Impacted Files Coverage Δ
astroquery/utils/mocks.py 84.61% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27515e4...0bd6724. Read the comment docs.

@bsipocz bsipocz merged commit 08e57d7 into astropy:main Feb 7, 2022
@bsipocz
Copy link
Member

bsipocz commented Feb 7, 2022

Thanks @eerovaher!

@bsipocz bsipocz added this to the v0.4.6 milestone Feb 7, 2022
@eerovaher eerovaher deleted the rm-testing-tools branch February 21, 2022 13:09
syed-gilani pushed a commit to syed-gilani/astroquery that referenced this pull request Mar 11, 2022
mhsarmiento pushed a commit to esdc-esac-esa-int/astroquery that referenced this pull request May 24, 2022
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.

turn_on/off internet should use pytest-remotedata?
3 participants