html5lib/html5lib-pythonPublic

NotificationsYou must be signed in to change notification settings
Fork302
Star1.2k

Support pytest 4#429

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

mcepl wants to merge2 commits intohtml5lib:masterfrommcepl:pytest4

Closed

Support pytest 4#429

mcepl wants to merge2 commits intohtml5lib:masterfrommcepl:pytest4

Conversation

Copy link

mcepl commentedNov 1, 2019

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

hugovk suggested changes

Nov 8, 2019

View reviewed changes

Copy link

Contributor

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 mentioned this pull request

Nov 8, 2019

Support pytest 4#414

Closed

Copy link

Author

hugovk commentedNov 8, 2019

Good, thanks! Flake8 now passes on Travis CI.

hugovk reviewed

Nov 8, 2019

View reviewed changes

requirements-test.txt OutdatedShow resolvedHide resolved

Copy link

codecov-io commentedNov 8, 2019•
edited
Loading

Codecov Report

Merging#429 intomaster willdecrease coverage by1.82%.
The diff coverage is71.42%.

@@            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%)`	⬇️
... and5 more

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last updateaf19281...29bb168. Read thecomment docs.

Copy link

felixonmars commentedNov 20, 2019

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

I changed the line

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

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 Mappinghtml5lib/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 =========================

Copy link

Contributor

jdufresne commentedJan 5, 2020

Would you consider to include the change too?

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

Copy link

Author

mcepl commentedJan 6, 2020•
edited
Loading

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 withpytest 4.6.6 andpytest 5.2.4.
.

Copy link

felixonmars commentedJan 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.

Copy link

Author

mcepl commentedJan 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?

gsnedders mentioned this pull request

Feb 26, 2020

Fix Pytest4.x compatibility error#420

Merged

gsnedders reviewed

Feb 26, 2020

View reviewed changes

Copy link

Member

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 becomesdef 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 fromrunParserEncodingTest?)

html5lib/tests/test_encoding.py

		assertencoding==stream.charEncoding[0].name,errorMessage(data,encoding,stream.charEncoding[0].name)


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

Copy link

Member

gsneddersFeb 26, 2020

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.pyShow resolvedHide resolved

html5lib/tests/test_sanitizer.py OutdatedShow resolvedHide resolved

html5lib/tests/test_stream.pyShow resolvedHide resolved

html5lib/tests/test_treewalkers.py Outdated

		fortreeinsorted(treeTypes.items()):
		forintext,attrs,expectedinsm_tests:
		yieldrunTreewalkerEditTest,intext,expected,attrs,tree
		runTreewalkerEditTest(intext,expected,attrs,tree)