Skip to content
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

Fixed and refactored roundtrip, n3_suite and nt_suite tests #1644

Merged

Conversation

aucampia
Copy link
Member

@aucampia aucampia commented Jan 2, 2022

Fixed and refactored roundtrip, n3_suite and nt_suite tests

Issues in previous roundtrip that are fixed:

  1. Roundtrip tests were skipping tests that would fail instead of
    reporting them with xfail.
    While this is maybe a minor distinction it
    is more appropriate to report as xfail as that is a better semantic
    description of a known issue. Skipping should be reserved for tests
    that are expected to succeed when ran but can only run in a suitable
    environment (e.g. if berkleydb is installed). Furthermore, marking
    tests with xfail will still run the test, and will also report them
    as unexpected pass (xpass) if they do actually pass.

  2. Removed ("json-ld", "example-misc.n3") from xfail/skip as it does
    pass.

Refactoring and improvements:

  1. Removed test/test_n3_suite.py::test_n3_writing.
    This test was doing something functionally equivalent to what
    test/test_roundtrip.py::test_n3 would do, except it was doing it
    with different code (test/testutils.py::check_serialize_parse).
    Specifically it would test, parse from n3, serialize to n3,
    parse from n3, and this is also something that
    test/test_roundtrip.py::test_n3 does. One difference is that
    check_serialize_parse was using crapCompare instead of
    rdflib.compare.isomorphic, and crapCompare does a significantly
    worse job but also hides an issue with rdflib.compare.isomorphic
    and quoted graphs which is now correctly reported as xfail.
  2. Removed _get_test_files_formats and all_{n3,nt}_files from
    test/test_{nt,n3}_suite.py (and removed the files since there is
    nothing else in them of value). These are now replaced with code
    inside test/test_roundtrip.py which has less duplication and uses
    guess_format for determinering formats instead of a custom if/elif
    block.
  3. Change skipping/xfail to work on file basename instead of path.
  4. Changed file collector from a generator to a function returning a
    list as the result of this function is iteratred over multiple times.
  5. Changed test id's to be more meningful, e.g. roundtrip_n3-writer-test-28.n3_n3_trig
    instead of roundtrip-args303.

Other changes:

  1. Renamed test/nt/test.ntriples -> test/nt/test.nt: .ntriples is not
    a recognized extension and the test this is being used in does not
    test anything pertaining to extention handling.
  2. Add guess_format tests for .{rdf,nt,n3} and negative tests for
    .{docx,mkv} and a file with no extension.

@aucampia
Copy link
Member Author

aucampia commented Jan 2, 2022

Diff in coverage:

  Module                                                                                            | statements | missing | excluded | branches | partial | coverage 
1c1
< # Coverage report: 81%
---
> # Coverage report: 82%
13c13
< [rdflib/compare.py](d_9b73ddaf1c733d26_compare_py.html)                                           | 336        | 18      | 0        | 183      | 11      | 93%      
---
> [rdflib/compare.py](d_9b73ddaf1c733d26_compare_py.html)                                           | 336        | 18      | 0        | 183      | 10      | 93%      
23c23
< [rdflib/graph.py](d_9b73ddaf1c733d26_graph_py.html)                                               | 910        | 178     | 2        | 424      | 42      | 79%      
---
> [rdflib/graph.py](d_9b73ddaf1c733d26_graph_py.html)                                               | 910        | 177     | 2        | 424      | 42      | 79%      
51c51
< [rdflib/namespace/__init__.py](d_d11ffb0fa4573d19___init___py.html)                               | 379        | 33      | 0        | 156      | 15      | 90%      
---
> [rdflib/namespace/__init__.py](d_d11ffb0fa4573d19___init___py.html)                               | 379        | 30      | 0        | 156      | 15      | 90%      
68c68
< [rdflib/plugins/serializers/jsonld.py](d_07086a33ade23e2f_jsonld_py.html)                         | 227        | 17      | 0        | 149      | 17      | 90%      
---
> [rdflib/plugins/serializers/jsonld.py](d_07086a33ade23e2f_jsonld_py.html)                         | 227        | 17      | 0        | 149      | 16      | 90%      
73c73
< [rdflib/plugins/serializers/rdfxml.py](d_07086a33ade23e2f_rdfxml_py.html)                         | 211        | 12      | 0        | 116      | 14      | 92%      
---
> [rdflib/plugins/serializers/rdfxml.py](d_07086a33ade23e2f_rdfxml_py.html)                         | 211        | 13      | 0        | 116      | 15      | 91%      
76c76
< [rdflib/plugins/serializers/turtle.py](d_07086a33ade23e2f_turtle_py.html)                         | 289        | 24      | 0        | 124      | 10      | 89%      
---
> [rdflib/plugins/serializers/turtle.py](d_07086a33ade23e2f_turtle_py.html)                         | 289        | 23      | 0        | 124      | 9       | 90%      
88c88
< [rdflib/plugins/sparql/evaluate.py](d_b85a33f12b1281b4_evaluate_py.html)                          | 315        | 18      | 0        | 193      | 13      | 94%      
---
> [rdflib/plugins/sparql/evaluate.py](d_b85a33f12b1281b4_evaluate_py.html)                          | 315        | 17      | 0        | 193      | 12      | 94%      
116c116
< [rdflib/term.py](d_9b73ddaf1c733d26_term_py.html)                                                 | 697        | 127     | 3        | 379      | 39      | 79%      
---
> [rdflib/term.py](d_9b73ddaf1c733d26_term_py.html)                                                 | 697        | 125     | 3        | 379      | 39      | 79%      
126c126
< Total                                                                                             | 20297      | 2994    | 127      | 7271     | 761     | 81%      
---
> Total                                                                                             | 20297      | 2987    | 127      | 7271     | 758     | 82%      

generated with:

# executed in each dir
.venv/bin/python3 -m pytest --cov-report html --cov

# diff
html2text -b 999 --pad-tables htmlcov/index.html | \
  sed -n 's/^.*statements.*missing.*$/  &/gp' ; \
  diff \
    <(html2text -b 999 --pad-tables ../rdflib.master/htmlcov/index.html | grep -v 'created at') \
    <(html2text -b 999 --pad-tables htmlcov/index.html | grep -v 'created at')

@aucampia aucampia force-pushed the iwana-20220101T2329-test_roundtrip branch from fbe8607 to 1e20216 Compare January 2, 2022 11:50
Issues in previous roundtrip that are fixed:

1. Roundtrip tests were skipping tests that would fail instead of
   reporting them with xfail.
   While this is maybe a minor distinction it
   is more appropriate to report as xfail as that is a better semantic
   description of a known issue. Skipping should be reserved for tests
   that are expected to succeed when ran but can only run in a suitable
   environment (e.g. if berkleydb is installed). Furthermore, marking
   tests with xfail will still run the test, and will also report them
   as unexpected pass (xpass) if they do actually pass.

2. Removed ("json-ld", "example-misc.n3") from xfail/skip as it does
   pass.

Refactoring and improvements:

1. Removed `test/test_n3_suite.py::test_n3_writing`.
   This test was doing something functionally equivalent to what
   `test/test_roundtrip.py::test_n3` would do, except it was doing it
   with different code (`test/testutils.py::check_serialize_parse`).
   Specifically it would test, parse from n3, serialize to n3,
   parse from n3, and this is also something that
   `test/test_roundtrip.py::test_n3` does. One difference is that
   `check_serialize_parse` was using `crapCompare` instead of
   `rdflib.compare.isomorphic`, and `crapCompare` does a significantly
   worse job but also hides an issue with `rdflib.compare.isomorphic`
   and quoted graphs which is now correctly reported as xfail.
2. Removed `_get_test_files_formats` and `all_{n3,nt}_files` from
   `test/test_{nt,n3}_suite.py` (and removed the files since there is
   nothing else in them of value). These are now replaced with code
   inside `test/test_roundtrip.py` which has less duplication and uses
   `guess_format` for determinering formats instead of a custom if/elif
   block.
3. Change skipping/xfail to work on file basename instead of path.
4. Changed file collector from a generator to a function returning a
   list as the result of this function is iteratred over multiple times.
5. Changed test id's to be more meningful, e.g. `roundtrip_n3-writer-test-28.n3_n3_trig`
   instead of `roundtrip-args303`.

Other changes:

1. Renamed `test/nt/test.ntriples` -> `test/nt/test.nt`: .ntriples is not
  a recognized extension and the test this is being used in does not
  test anything pertaining to extention handling.
2. Add guess_format tests for `.{rdf,nt,n3}` and negative tests for
  `.{docx,mkv}` and a file with no extension.
@aucampia aucampia force-pushed the iwana-20220101T2329-test_roundtrip branch from 1e20216 to e8fe7e1 Compare January 2, 2022 12:05
@aucampia aucampia marked this pull request as ready for review January 2, 2022 12:10
@nicholascar
Copy link
Member

Glad @aucampia got rid of crapCompare! I've seen it in there for a while and figured that isomorphic must have come along much later.

@nicholascar nicholascar self-requested a review January 2, 2022 23:07
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.

Merging after one review as fair straightforward PR

@nicholascar nicholascar merged commit 1226b52 into RDFLib:master Jan 2, 2022
@aucampia aucampia deleted the iwana-20220101T2329-test_roundtrip branch January 8, 2022 10:13
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.

None yet

2 participants