Migrate from nosetest to pytest #1452
Conversation
aucampia
force-pushed
the
iwana-20211018T2122-pytest
branch
from
41dd7f9 to
33981f5
Compare
aucampia
force-pushed
the
iwana-20211018T2122-pytest
branch
10 times, most recently
from
dacd663 to
9e4e948
Compare
| doctest-ignore-unicode==0.1.2 | ||
| flake8 | ||
| flake8-black | ||
| html5lib |
There was a problem hiding this comment.
html5lib is added because it is in setup.py requirements for tests and it is actually being used in one place in rdflib though I don't think it is actually used by tests. Fine to remove it also.
| @@ -1,9 +0,0 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
This was only used by docker stuff, but now I'm just invoking pytest directly from there.
| @@ -1,9 +0,0 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
This was only used by docker stuff, but now I'm just invoking pytest directly from there.
| "nose==1.3.7", | ||
| "nose-timer", | ||
| "coverage", | ||
| "black==21.9b0", |
There was a problem hiding this comment.
I removed black and flake8 as they are not used for testing, don't mind adding them back, but I think this should either be everything from requirements.dev.txt or just stuff needed for testing.
aucampia
force-pushed
the
iwana-20211018T2122-pytest
branch
from
e13b785 to
fa9f85e
Compare
aucampia
force-pushed
the
iwana-20211018T2122-pytest
branch
2 times, most recently
from
3d3a22e to
aab698d
Compare
This patch replace all uses of nose with pytest. It also includes a
pytest plugin for creating EARL reports for tests with a `rdf_test_uri`
parameter.
Some caveats:
- HTML report directory is now htmlcov instead of coverage
- There is some warning related to the EARL reporting plugin which I can't quite figure out:
```
.venv/lib64/python3.7/site-packages/_pytest/config/__init__.py:676
/home/iwana/sw/d/github.com/iafork/rdflib/.venv/lib64/python3.7/site-packages/_pytest/config/__init__.py:676: PytestAssertRewriteWarning: Module already imported so cannot be rewritten: test.earl
self.import_plugin(import_spec)
```
This is not causing any problems as far as I can tell, but still annoying.
- python setup.py test won't work anymore, I can make it work but this
is not advised by pytest:
https://github.com/pytest-dev/pytest-runner/#deprecation-notice
- run_test.py is still there but it's not really referenced anymore from
anywhere and the options it accepts are completely different as it's options
were based on nose. I would say it should be removed entirely but for now
it is basically just a wrapper around pytest that basically does nothing.
- Removed references to test attributes as currently they are not being
used anywhere anyway, I guess we can add them back if there is some
use for them later.
- A lot of tests are still marked to skip when really they should be marked
with xfail. This is also affecting the RDFT test manifests and result in
reports saying tests are skipped when really we know they will fail and
they are only skipped for this reason. But there is no change here from
before, and pytest makes it easier to dynamically do expected failures.
Special thanks to Wes Turner for his advice and inputs on this process.
aucampia
force-pushed
the
iwana-20211018T2122-pytest
branch
from
aab698d to
c2a37b7
Compare
- Removed unused ignores. - Use official description text of RDFLib in DOAP used in EARL report.
@white-gecko you don't maybe have something related to Cython going on? The only thing I can find related to "build_build_ext" which I can find seems to be that: |
|
Cleaning the Now I get |
Looks about right compared to drone results. Still working on making tests pass on windows, to add another thing to the list of things broken on windows: the TSV result parser does not work on text files on windows as it does not handle |
Just to be clear, I think this should be merged without fixes for windows, as those should really be treated as fixes in their own right given tests do not work on master under windows anyway, but working on it as at some point it would be nice to get some of the issues fixed, and the start to that is having tests that can actually run. If there is still on this when I'm done with it I will add to this PR together with changes to run CI under windows and MacOS using GitHub actions. |
Agreed: we can address Windows issues later. I still can’t install BerkeleyDB on Windows (since I can’t compile 2015 C++) so there are a few things to fix.
Thanks! However I would like to keep track of all the tests that can be skipped so that it’s easier to know what is and isn’t tested. I think pytest presents this info better than nose already. Elsewhere I would like to get the BerkeleyDB tests running on Windows, but that’s more part of updating all the Stores, in particular the SQLite Store. |
|
On a Mac with the BerkeleyDB tests all running:
The 7 errors are all due to Can't assign requested address, 5 of the 7 failures are also related to that and the remaining two are HTTPMock failures which I think are also also related to that! So all the failures & errors are all related to the same thing. What was the trick to allow it to assign an address? Running as sudo or something? Also, what else should I be doing to reduce the number of skipped tests? On a Windows machine, without BerkeleyDB:
All the test_dawg_data_sparql11 failed, just as @aucampia says, so just confirming that. The other failures seem to be Windows path related etc. So, leaving aside the Windows results, my Mac results are close to yours. |
@nicholascar please do share the output, I will see what I can do about it. I will try setup pipelines with GitHub actions on my branch shortly to see if that reproduces problems with MacOS. |
|
Latest run results with Fuseki running to act as a read/write SPARQL endpoint: Strange! So 4 more failures - auth errors, see the results details below - but the same number of passed and skipped & errors. If there are 4 more failures, I expect to see 4 fewer passed or errors?? Here's my output: So it looks like all 11 of my errors (or 7, without Fuseki (!)) are a result of some OS port assignment thing and therefore "don't count" regarding evaluating this PR. I'll try to reduce the numbers of skipped tests next |
I figured out what is wrong with these, macos does not like the random loopback IP addresses. I will add a commit to always use |
This is so that it works properly on MacOS which does not permit listening on random loopback addresses for user processes.
Oh no! I installed Here's my summary of skipped tests (using OK, clearly I'm able to get the skipped count down to 67 and, with different config, get 3887 tests passed - not quite 3901 but close! I'll pass this now and leave final testing approval to @white-gecko (and perhaps @ashleysommer - please test!!). |
Yes, that's right. I remembered that I've seen this before so, as I say, this is a Mac thing, not an RDFlib problem really!
That would be cool |
Errors were being piped to true, but that will fail if it runs with pipefail. The better option is to do `black ... || true` which will work for ignoring errors even with pipefail.
|
Added two more commits: c3270b1 - Change mock HTTP server to listen on 127.0.0.1 by defaultThis is so that it works properly on MacOS which does not permit 5393e0f - Fix how black errors are ingored in .drone.yamlErrors were being piped to true, but that will fail if it runs with |
|
I setup a GitHub actions workflow to test on macos, results can be seen here: https://github.com/iafork/rdflib/actions?query=branch%3Aiwana-20211113T1312-github-actions The branch is essentially the same as this one just with some changes for making github actions work: aucampia/rdflib@iwana-20211018T2122-pytest...iwana-20211113T1312-github-actions Did not do much to reduce skipped tests, will maybe look at that later. May be worth enabling github actions on main repo, at the very least we can use it for additional validation but I think it is likely better than drone in most ways and runs macos, windows and linux. |
Got it to this now: |
Okay got it to pass even more on MacOS now: https://github.com/iafork/rdflib/runs/4218497302?check_suite_focus=true Not sure things will get much better than this. |
|
Yes, I think that's going to be as good as it gets too! @ashleysommer @white-gecko I've already indicated 'Approve'. Can one or other of you please do likewise and we will merge in. Then we will need to instantly raise an issue to address the DAWG tests. |
I am almost done fixing issues on windows: aucampia/rdflib@iwana-20211018T2122-pytest...iwana-20211116T1039-fix_windows I already have all DAWG tests passing there and just have like 50 failing tests left which I will try and fix soon. |
|
@aucampia your contributions to RDFlib recently have been truly inspiring! |
I'm already in the middle of the review but not yet had the chance to finish it. I hope I will complete it soon. |
white-gecko
left a comment
white-gecko
left a comment
There was a problem hiding this comment.
This is a huge pull request. I could not check each individual line. Overall it looks good and it is working as expected. I'm very impressed by your work @aucampia, thank you for this huge contribution!
|
Great work @aucampia indeed! I will merge in tomorrow when I’m back at my computer (not phone like this!) |
|
Glad to help out @white-gecko and @nicholascar - will hopefully have a PR for windows fixes this weekend and I will make an issue for running MacOS and windows pipelines so we can avoid issues in future where things break on one platform. |
3 participants
This patch replace all uses of nose with pytest. It also includes a
pytest plugin for creating EARL reports for tests with a
rdf_test_uriparameter.
Some caveats:
There is some warning related to the EARL reporting plugin which I can't quite figure out:This is not causing any problems as far as I can tell, but still annoying.(this problem has been fixed in the latest commit)
is not advised by pytest:
https://github.com/pytest-dev/pytest-runner/#deprecation-notice
anywhere and the options it accepts are completely different as it's options
were based on nose. I would say it should be removed entirely but for now
it is basically just a wrapper around pytest that basically does nothing.
used anywhere anyway, I guess we can add them back if there is some
use for them later.
with xfail. This is also affecting the RDFT test manifests and result in
reports saying tests are skipped when really we know they will fail and
they are only skipped for this reason. But there is no change here from
before, and pytest makes it easier to dynamically do expected failures.
Special thanks to Wes Turner for his advice and inputs on this process.
Other info
Fixes #1414
Closes #1268
The work in this PR is based on #1268 which was started by @white-gecko.
Coverage can be seen at https://coveralls.io/github/RDFLib/rdflib
Proposed Changes