Closed johnomotani closed 4 years ago
Hello @johnomotani! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:
Merging #160 into master will increase coverage by
0.00%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #160 +/- ##
=======================================
Coverage 76.89% 76.89%
=======================================
Files 11 12 +1
Lines 1913 1939 +26
Branches 431 437 +6
=======================================
+ Hits 1471 1491 +20
- Misses 291 296 +5
- Partials 151 152 +1
Impacted Files | Coverage Δ | |
---|---|---|
conftest.py | 76.92% <100.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 3b20d69...dc41fee. Read the comment docs.
I think this PR is an improvement, but I'd be glad to know of a neater way to do it... The problem it's trying to solve is to get pytest
configuration set up in a way that works both: (1) after cloning the git repo, running pytest
from either the top level, or in the xbout
directory, etc; and (2) when xbout is installed as a package, running pytest --pyargs xbout
.
The problems:
pytest.ini
isn't used by pytest
unless it's in or above the directory where you run pytest
, so the pytest --pyargs xbout
version fails even if we installed pytest.ini
with the package.conftest.py
is used when installed locally (which is good for us), but is apparently a bit too local. (Just) putting conftest.py
under the xbout/
directory so that it gets installed with the package means that running pytest
at the top level of the git repo fails if you want to use the --long
argument - apparently pytest
parses the command line arguments before it loads the configuration from subdirectories.conftest.py
in the top level of the repo, but have it installed within the xbout
package (this might be the neatest solution if it was possible).The solutions:
pytest.ini
problem is fairly simply fixed by doing all the configuration in conftest.py
. It's ugly (there's a separate function call for every line that was in the pytest.ini
) but it works.conftest.py
in the top level of the repo, and another one in the xbout/
directory. The one in the top level imports the setup functions that pytest
uses from xbout/conftest.py
, which is only slightly ugly. There's a new problem though, because like this running pytest
from the top level of the repo (or actually anywhere in the git repo, apparently it manages to go back up to make the top level its root directory) means the setup gets imported twice and so pytest
fails with an error about argument '--long' is already defined
(or something roughly like that). To get around that, I've added some global flags to xbout/conftest.py
which are False
to start with and switched to True
the first time the set-up functions are called and then stop the functions doing anything the second time they are called. Just to make this bodge lovelier, when I tried using the same flag for all three functions it didn't work, so there's a separate flag for each function.So in summary, this solution seems to work. I think it's ugly, and I'd welcome a nicer way, but I think it's better to have some solution that makes --long
work both in a cloned xbout
git repo and for xbout
installed as a package.
PS the original motivation for this is that the long
marker was not being recognised by the conda-forge
set up, so that was running the long version of the tests (which seems like an anti-social thing to do when they take ~30 mins to run, and the package was already tested by the Github action before being released).
I want to get xbout
released on conda, so I'm going to merge this now. If anyone has nicer ideas, we can rework in another PR.
pytest.ini is ignored even if installed when testing the installed package with
pytest --pyargs xbout
. To get around this, set all options inconftest.py
, and moveconftest.py
to thexbout/
directory so that it gets installed with the package.Note, putting
conftest.py
in thexbout/
directory so that it is used both bytests/*
andcalc/tests/*
.