Repository navigation
Testing/test runner refactor - #4020
Conversation
|
so sunpy has a subclass of the https://github.com/sunpy/sunpy/blob/master/sunpy/tests/setup_command.py |
|
@Cadair That's right. You won't have to make that change immediately though. The old command class will be kept for backwards compat for now. Maybe instead though you could describe what you mean by "fiddle the online offline options". Could that just be included in the test runner? |
e825a7e to
34e9691
Compare
|
So, when I moved sunpy over to ah the testing infrastructure was a bit of a botch, I wanted to keep our previous online mark behaviour from pytest i.e. I was discussing with @bsipocz that our runner probably needs re-writing because we seem to be having some issues with the doctest build stalling on travis, and as I said it's a bit of a hack. How would I change these options from the test runner? |
|
@Cadair - or there may be no need to rewrite stuff if it will be easy to just add extra options (like the planned Thus I guess all the astropy doctest directives would work out of the box and solve sunpy/sunpy#1456 and the halting builds, etc. |
|
This should make it easier to add new options, yes. On the astropy-helpers end of things, it will work so that even if you don't have astropy available there is still a dummy "test" command, so users can see that it's available. It just won't work if astropy isn't installed (and it says as much if you try to use it). Also, @mdboom and I were talking possibly at some point pulling the astropy testing helpers out to a separate package (not part of astropy-helpers, but something else). I don't have an ETA on that though. |
There was a problem hiding this comment.
For the affiliated package template we may need something slightly ugly here along the lines of:
try:
from astropy.tests.runner import TestRunner
test = TestRunner.make_test_runner_in(__path__[0])
except ImportError:
def test(*args, **kwargs):
"""Run the package test suit. Requires the astropy package to be installed."""
raise RuntimeError("The astropy package must be installed in order to run the tests.")
|
So how does this make it easier to add new options to the TestHelper for flags to setup.py? |
|
Did you mean "for" or "or"? Adding options to setup.py is still treated independently from the TestRunner class, for a number of reasons. But by being in the same package it means all options added to the |
|
In other words, it's "easier" in the sense that you only have to make changes in one or two places (all in If you want to subclass you just have to subclass |
|
If there are no objections, this should be included in v1.1.0. To be clear: this doesn't have any effect in terms of backwards-compatibility. |
6fc03ff to
d16f9ff
Compare
|
[rebased on master] |
|
Hmm, not clear at all why this blew up so badly. Need to rebase again, then we'll see... |
d16f9ff to
ea172ba
Compare
ea172ba to
c388b90
Compare
|
@embray - can you rebase? Do you want to get this into 1.1? |
c388b90 to
9d82db6
Compare
|
Rebased and fixed indentation problem. |
40ece05 to
f161502
Compare
going on here: 1) The TestRunner class has been moved out of astropy.tests.helper into its own module astropy.tests.runner. 2) The astropy.tests.__init__ module has been cleaned up so that there isn't anything imported into the top level of the astropy.tests namespace. In particular astropy.tests.helper is not imported. This is so that astropy.tests.runner can be imported without causing pytest to be imported too. 3) Added a TestRunner.make_test_runner_in classmethod--this actually *creates* the test() function that is called as astropy.test(). It gets its docstring and arguments from TestRunner.run_tests (this is in contrast to the previous situation where TestRunner.run_tests and astropy.test got different information from each other, and arguments signatures had to be specified in two different places). 4) Added astropy.tests.command module containing the AstropyTest setup.py command implementation (moved back from astropy-helpers). In retrospect it was a mistake to keep this separately in astropy-helpers, because the functionality of the test command is very much tied to what capabilities are available in the test runner, so that having them in separate projects made it difficult to keep them in sync. An associated change will be added to astropy-helpers to add a wrapper that attempts to import the test command from astropy (and will provide a fallback if it is unavailable). 5) Changed how the astropy.__init__ namespace is cleaned up.
…and in any case the function we're wrapping doesn't *have* any annotations in the first place).
…eral previous fixes from other PRs. Also updated astropy.tests.command to incorporate the changes from astropy/astropy-helpers#190
…astropy-helpers#194 into astropy's copy of the AstropyTest command.
fe90935 to
db8aaa6
Compare
|
Wow, this is finally ready to merge \o/ Anyone want to review this before I do? |
There was a problem hiding this comment.
Should we consider this as being part of the public API and therefore keep the last import?
There was a problem hiding this comment.
No, it's never been documented as such. I don't think this import being here was ever particularly deliberate, and nobody seems to rely on it.
|
Apart from my small comment above, this is good to go 👍 |
4 participants
Following discussion in #2317, here's some changes that will make it I think a little easier to add changes to the
astropy.teststest runner. This also moves theAstropyTestdistutils command class back into astropy.tests so that it isn't awkwardly separated in astropy-helpers. There will be an associated change in astropy-helpers, so this PR should include a submodule update before it's merged.This PR depends on #4017.