ALMA integration tests fix#2224
Conversation
keflavich
left a comment
There was a problem hiding this comment.
Looks good.
Could you please add some documentation about the --alma-site testing to https://astroquery.readthedocs.io/en/latest/testing.html ?
Codecov Report
@@ Coverage Diff @@
## main #2224 +/- ##
==========================================
+ Coverage 61.81% 61.96% +0.15%
==========================================
Files 129 129
Lines 16295 16297 +2
==========================================
+ Hits 10072 10099 +27
+ Misses 6223 6198 -25
Continue to review full report at Codecov.
|
|
@keflavich , @bsipocz - I think this is ready. Please have a look. Thanks |
keflavich
left a comment
There was a problem hiding this comment.
lgtm, just have one URL-related question
bsipocz
left a comment
There was a problem hiding this comment.
Nitpick, except that we shouldn't disable rst doctesting. I'll add that commit, and then merge.
| """ | ||
| Returns an alma client class. `--alma-site` pytest option can be used | ||
| to have the client run against a specific site | ||
| :param request: pytest request fixture |
There was a problem hiding this comment.
doesn't matter here as we don't docstring test functions, but normally we use numpydoc style docstring formatting
|
|
||
|
|
||
| def pytest_addoption(parser): | ||
| parser.addoption( |
There was a problem hiding this comment.
again, this is nit pick, we don't use 'black', formatting so no need to put every arg in a new line 😄
| xfail_strict = true | ||
| remote_data_strict = true | ||
| addopts = --doctest-rst | ||
| #addopts = --doctest-rst |
There was a problem hiding this comment.
Please remove this comment, we started to doctest narrative docs for a couple of modules and eventually would like to see all of them being tested
|
I've also rebased to be able to add a changelog for the new pytest option. |
|
Thanks @andamian! |
Updates to the ALMA integration tests to use ALMA production servers.
A fix for #2157