Skip to content

Fix for remote tests: atomic#2215

Merged
bsipocz merged 2 commits into
astropy:mainfrom
keflavich:fix_atomic_tests_by_not_caching
Nov 17, 2021
Merged

Fix for remote tests: atomic#2215
bsipocz merged 2 commits into
astropy:mainfrom
keflavich:fix_atomic_tests_by_not_caching

Conversation

@keflavich

Copy link
Copy Markdown
Contributor

See #2203

The "fix" locally was to:

  1. specify the BeautifulSoup parser (this is OK)
  2. change the tests so they don't use caching. This is not OK, there shouldn't be caching in the tests anyway.

@codecov

codecov Bot commented Nov 17, 2021

Copy link
Copy Markdown

Codecov Report

Merging #2215 (c30a543) into main (a654433) will not change coverage.
The diff coverage is 50.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2215   +/-   ##
=======================================
  Coverage   61.80%   61.80%           
=======================================
  Files         129      129           
  Lines       16286    16286           
=======================================
  Hits        10065    10065           
  Misses       6221     6221           
Impacted Files Coverage Δ
astroquery/atomic/core.py 36.88% <50.00%> (ø)

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 a654433...c30a543. Read the comment docs.

@bsipocz bsipocz added this to the v0.4.4 milestone Nov 17, 2021
@bsipocz

bsipocz commented Nov 17, 2021

Copy link
Copy Markdown
Member

I dont' think point no1 needs a changelog, but please add one if you think otherwise.

And I totally agree with point no2, and don't see how and why and when we ended up using caching for the tests. Maybe it deserves a separate issue for it.

@bsipocz bsipocz mentioned this pull request Nov 17, 2021
28 tasks
@keflavich

Copy link
Copy Markdown
Contributor Author

OK I'm removing WIP, then.

I agree, no changelog needed.

I'm uncomfortable that I don't understand why caching is causing problems (that I can't reproduce when I do the tests interactively), but... let's merge this anyway.

@keflavich keflavich changed the title WIP: Fix for remote tests: atomic Fix for remote tests: atomic Nov 17, 2021
@bsipocz bsipocz merged commit 1b241d5 into astropy:main Nov 17, 2021
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