Skip to content

Fix tests and make them more reliable #244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 27, 2025
Merged

Conversation

vemonet
Copy link
Contributor

@vemonet vemonet commented Apr 22, 2025

@nicholascar There was many errors when running the tests, mostly due to public endpoints used for the tests not available anymore, or the tests were designed in a way that would break as soon as the data in an endpoint changes

I fixed them all

  • Comment out 2 tests that were targeting endpoints that are not available anymore. And a test that is targeting http://zbw.eu/beta/sparql/stw/query, but this endpoints errors on all POST queries but works on GET, might need to be investigated
  • Fix errors and warnings due to using deprecated rdflib ConjunctiveGraph instead of Dataset
  • Fix CLI tests failing due to slightly different result in response, now checking the length of results bindings, this will be more reliable in the future and is a good marker for a successful query. Asking for ?s ?p ?o LIMIT 1 on publicly available endpoints with potentially content evolving over time, and checking for an exact URI in the response is really surprising (to say the least, I cannot understand how anyone knowing a little bit about semweb and testing software thinks this will be a reliable test), it takes 20 lines of hardly readable code for each test, and breaks as soon as the endpoint content changes, now after fix, it takes 1 line per test, and will not break in the future (apart if the query fails but that's the whole point of the test)
  • Fix tests for error message by checking if the error is in the message (instead of checking only the end of the message, which was not working)
  • Added pytest and using it to run the tests. It is a more modern and reliable solution that works with the current code. unittest had a bug with the 3 tests where warnings are caught (when running each test alone, they work, but when running all with unittest catching the warnings stops working, but it works with pytest). I also actually fixed the bug, so now all tests works with pytest and unittest, but the trust in unittest is lost! Fixes Running all tests at once causes failure of test for Wrapper.py #192
  • Added python 3.11 and 3.12 to the github actions tests

It seems like the tests have been broken for a long time, especially considering these 2 years old errors that I fixed: #192

I hope this PR will help to enable maintainers to more easily test and merge the waiting and future PRs

- Fix test due to using deprecated rdflib ConjunctiveGraph instead of Dataset
- Comment out 3 tests that were targeting endpoints that are not available anymore. Apart for http://zbw.eu/beta/sparql/stw/query that errors on POST queries but works on GET, might need to be investigated
- Fix CLI tests failing due to slightly different result in response, now checking the length of results bindings, this will be more reliable in the future and is a good marker for a successful query
- Fix tests for error message by checking if the error is in the message (instead of checking only the end of the message)
- Added pytest and using it to run the tests. It is a more modern and reliable solution that works with the current code. unittest had a bug with the 3 tests where warnings are caught (when running each test alone, they work, but when running all with unittest catching the warnings stops working, but it works with pytest). I fixed the bug (so now all tests works with pytest and unittest, but the trust in unittest is lost). Fixes RDFLib#192
- Added python 3.11 and 3.12 to the github actions tests
@vemonet
Copy link
Contributor Author

vemonet commented Apr 23, 2025

In the second commit I could not resist to refactor the tests on public endpoints, they were such a pain to maintain, which explains why they were left broken so long

We now use pytest and parametrize the tests to public SPARQL endpoints (test_cli and test_wrapper still uses unittest but can be run with pytest).
So now instead of having 12 files of 1300+ lines each, we now have 1 file with 774 lines, with the same test suite as before 🤯

But this time you are sure that the exact same tests are run for every endpoints, instead of relying on 12 1000+ lines copy/paste from several years ago that is slightly edited at places

The test suite is now much more readable and maintainable, it is clearer which tests are run and skipped for each endpoint
When there is an error with one of the test it is easy to read for which endpoint and which parameters it is

Time to run the full test suite is between 8 and 15min depending on public endpoints responses time (worst are dbpedia and wikidata, some 60s+ calls). Also pytest will show the 3 longest durations tests at the end, so you know which one are problematic

Example tests running in gh actions: https://github.com/vemonet/sparqlwrapper/actions/runs/14621657263

This fixes #215

@nicholascar @aucampia

vemonet added 6 commits April 23, 2025 15:42
…Instead of having ~10 files of 1k3+ lines each, we now have 1 file with 774 lines, with the same test suite.

This makes the tests much more readable and maintainable. It is clearer which tests are run and skipped for each endpoint
Time to run the full test suite is ~9/15min depending on public endpoints responses time (worst are dbpedia and wikidata, some 60s+ calls)

Fixes RDFLib#215
…thon. update pytest-cov version to work with python 3.8
Copy link
Member

@nicholascar nicholascar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much for the major update!

Approving to run the automated tests.

@vemonet
Copy link
Contributor Author

vemonet commented Apr 25, 2025

Thanks @nicholascar! the failed tests are due to https://ja.dbpedia.org/sparql going down after the 3rd test (still down right now, 502 bad gateway)

Overall this is a strength and a problem with this test suite, it uses public endpoints.

I think moving to deploying test endpoint locally is not a good solution though, because it will add a lot of complexity (config each endpoint) + some endpoints are not deployable locally easily (stardog). And you want to actually test SPARQLWrapper in real conditions (public endpoints have proxies that are breaking things, you want SPARQLWrapper to handle this)

So the best solution I guess would be to have some basic checks that just check if the URL itself is available, and skip test if the whole URL is not available at all

@nicholascar nicholascar merged commit 5021dc1 into RDFLib:master Apr 27, 2025
0 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running all tests at once causes failure of test for Wrapper.py
2 participants