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

Support pytest 4 #429

Closed
wants to merge 2 commits into from
Closed

Support pytest 4 #429

wants to merge 2 commits into from

Conversation

mcepl
Copy link

@mcepl mcepl commented Nov 1, 2019

Rebase of #414 by @hroncok on the top of the current master, and two small nits fixed.

Copy link
Contributor

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Please could you fix the flake8 errors?

./html5lib/tests/test_sanitizer.py:71:20: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:72:20: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:75:20: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:76:20: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:79:20: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:80:20: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:83:20: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:84:20: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:97:16: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:98:16: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:105:16: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:106:16: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:114:16: E128 continuation line under-indented for visual indent
./html5lib/tests/test_sanitizer.py:115:16: E128 continuation line under-indented for visual indent

@hugovk hugovk mentioned this pull request Nov 8, 2019
@mcepl
Copy link
Author

mcepl commented Nov 8, 2019

Better?

@hugovk
Copy link
Contributor

hugovk commented Nov 8, 2019

Good, thanks! Flake8 now passes on Travis CI.

requirements-test.txt Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Nov 8, 2019

Codecov Report

Merging #429 into master will decrease coverage by 1.82%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #429      +/-   ##
==========================================
- Coverage   90.52%   88.69%   -1.83%     
==========================================
  Files          50       50              
  Lines        6973     6952      -21     
  Branches     1328     1308      -20     
==========================================
- Hits         6312     6166     -146     
- Misses        502      603     +101     
- Partials      159      183      +24
Impacted Files Coverage Δ
html5lib/tests/test_stream.py 93.82% <ø> (-5.06%) ⬇️
html5lib/tests/test_serializer.py 92.75% <100%> (ø) ⬆️
html5lib/tests/test_treewalkers.py 97.05% <100%> (ø) ⬆️
html5lib/tests/test_encoding.py 75% <33.33%> (-25%) ⬇️
html5lib/tests/test_sanitizer.py 94.59% <77.77%> (ø) ⬆️
html5lib/_inputstream.py 74.32% <0%> (-17.6%) ⬇️
html5lib/tests/test_meta.py 88% <0%> (-12%) ⬇️
html5lib/_utils.py 78.12% <0%> (-6.25%) ⬇️
... and 5 more

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 af19281...29bb168. Read the comment docs.

@felixonmars
Copy link

It fails here with pytest 5.2.4, perhaps pytest 5 changed again?

I changed the line

mark=pytest.mark.skipif(sys.maxunicode == 0xFFFF,

to

marks=pytest.mark.skipif(sys.maxunicode == 0xFFFF,

and it works fine now. Would you consider to include the change too?

The error:

==================================== ERRORS ====================================
________________ ERROR collecting html5lib/tests/test_stream.py ________________
html5lib/tests/test_stream.py:311: in <module>
    ???
/usr/lib/python3.8/site-packages/_pytest/mark/__init__.py:33: in param
    return ParameterSet.param(*values, **kw)
E   TypeError: param() got an unexpected keyword argument 'mark'
=============================== warnings summary ===============================
html5lib/_trie/_base.py:3
  /build/python-html5lib/src/html5lib-python-1.0.1/html5lib/_trie/_base.py:3: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.9 it will stop working
    from collections import Mapping

html5lib/treebuilders/dom.py:4
  /build/python-html5lib/src/html5lib-python-1.0.1/html5lib/treebuilders/dom.py:4: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.9 it will stop working
    from collections import MutableMapping

-- Docs: https://docs.pytest.org/en/latest/warnings.html
=========================== short test summary info ============================
FAILED html5lib/tests/test_stream.py - TypeError: param() got an unexpected k...
!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!
========================= 2 warnings, 1 error in 0.48s =========================

@jdufresne
Copy link
Contributor

Would you consider to include the change too?

That change makes sense to me. Can you go ahead with that?

@mcepl
Copy link
Author

mcepl commented Jan 6, 2020

Not sure, what’s wrong now. Is it some bug inside of pytest? Do I do something wrong?

Locally, it passes without any problem both with pytest 4.6.6 and pytest 5.2.4.
.

@felixonmars
Copy link

The original non-decorator pytest.mark.skipif line looks wrong to me from the beginning. It should be a decorated @pytest.mark.skipif in the next line IMHO.

@mcepl
Copy link
Author

mcepl commented Jan 6, 2020

The original non-decorator pytest.mark.skipif line looks wrong to me from the beginning. It should be a decorated @pytest.mark.skipif in the next line IMHO.

OK, I am too stupid and too sleepy, right now. Could I get a proper diff of what I should do, please?

Copy link
Member

@gsnedders gsnedders left a comment

Choose a reason for hiding this comment

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

The majority of these functions that yield tests all yield the same test function, right? Can we not just rename those functions, rename the functions they call, and call the existing functions with @pytest.mark.parametrize?

i.e., def test_encoding becomes def generate_encoding, the first argument from the yields is dropped, and then we end up with:

@pytest.mark.parameterize("data, encoding", generate_encoding())
def test_parser_encoding(data, encoding)

(renamed from runParserEncodingTest?)

@@ -95,12 +95,13 @@ def runPreScanEncodingTest(data, encoding):
assert encoding == stream.charEncoding[0].name, errorMessage(data, encoding, stream.charEncoding[0].name)


@pytest.mark.skip(reason="broken under pytest4")
Copy link
Member

Choose a reason for hiding this comment

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

This will need to be fixed before we can move to 4.x

html5lib/tests/test_sanitizer.py Show resolved Hide resolved
html5lib/tests/test_sanitizer.py Outdated Show resolved Hide resolved
html5lib/tests/test_stream.py Outdated Show resolved Hide resolved
@@ -99,7 +99,7 @@ def test_treewalker_six_mix():

for tree in sorted(treeTypes.items()):
for intext, attrs, expected in sm_tests:
yield runTreewalkerEditTest, intext, expected, attrs, tree
runTreewalkerEditTest(intext, expected, attrs, tree)
Copy link
Member

Choose a reason for hiding this comment

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

As above, we do want these as separate tests.

tox.ini Outdated Show resolved Hide resolved
This was referenced Feb 26, 2020
gsnedders added a commit to gsnedders/html5lib-python that referenced this pull request Feb 27, 2020
gsnedders added a commit to gsnedders/html5lib-python that referenced this pull request Feb 27, 2020
gsnedders added a commit that referenced this pull request Feb 27, 2020
@gsnedders gsnedders mentioned this pull request May 24, 2020
@gsnedders
Copy link
Member

Fixed by #497

@gsnedders gsnedders closed this Jun 9, 2020
@mcepl mcepl deleted the pytest4 branch June 9, 2020 09: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

7 participants