Python-Markdown / markdown

A Python implementation of John Gruber’s Markdown with Extension support.
https://python-markdown.github.io/
BSD 3-Clause "New" or "Revised" License
3.71k stars 856 forks source link

3.5.2: pytest fails #1429

Closed kloczek closed 7 months ago

kloczek commented 7 months ago

I'm packaging your module as an rpm package so I'm using the typical PEP517 based build, install and test cycle used on building packages from non-root account.

Here is pytest output: ```console r/lib/python3.8/site-packages + /usr/bin/pytest -ra -m 'not network' ==================================================================================== test session starts ==================================================================================== platform linux -- Python 3.8.18, pytest-7.4.4, pluggy-1.3.0 rootdir: /home/tkloczko/rpmbuild/BUILD/markdown-3.5.2 collected 1111 items / 1 error ========================================================================================== ERRORS =========================================================================================== _____________________________________________________________ ERROR collecting tests/test_syntax/extensions/test_md_in_html.py ______________________________________________________________ /usr/lib/python3.8/site-packages/_pytest/runner.py:341: in from_call result: Optional[TResult] = func() /usr/lib/python3.8/site-packages/_pytest/runner.py:372: in call = CallInfo.from_call(lambda: list(collector.collect()), "collect") /usr/lib/python3.8/site-packages/_pytest/python.py:814: in collect self.warn( /usr/lib/python3.8/site-packages/_pytest/nodes.py:304: in warn warnings.warn_explicit( E pytest.PytestCollectionWarning: cannot collect test class 'TestSuite' because it has a __init__ constructor (from: tests/test_syntax/extensions/test_md_in_html.py) ================================================================================== short test summary info ================================================================================== ERROR tests/test_syntax/extensions/test_md_in_html.py::TestSuite - pytest.PytestCollectionWarning: cannot collect test class 'TestSuite' because it has a __init__ constructor (from: tests/test_syntax/extensions/test_md_in_html.py) !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! ===================================================================================== 1 error in 0.54s ====================================================================================== ```
List of installed modules in build env: ```console Package Version ------------------ ------- build 1.0.3 cppclean 0.13 distro 1.8.0 dnf 4.18.2 exceptiongroup 1.1.3 gpg 1.23.2 importlib-metadata 7.0.1 iniconfig 2.0.0 installer 0.7.0 libdnf 0.72.0 packaging 23.2 pluggy 1.3.0 pyproject_hooks 1.0.0 pytest 7.4.4 python-dateutil 2.8.2 PyYAML 6.0.1 setuptools 69.0.3 six 1.16.0 tomli 2.0.1 wheel 0.42.0 zipp 3.17.0 ```
mitya57 commented 7 months ago

Try using plain unittest instead of pytest:

python -m unittest discover -v
kloczek commented 7 months ago

Correctly written unittest based test suite can be correctly handled by pytest.

waylan commented 7 months ago

It appears that Pytest is tripping over our use of the load_tests Protocol here:

https://github.com/Python-Markdown/markdown/blob/08dacae618775831243f6bbab47d9be590d511f2/tests/test_syntax/extensions/test_md_in_html.py#L1210-L1216

In this module, we import the TestHTMLBlocks unittest class and in a subclass override the extensions to include the md_in_html extension. In that way those tests get run a second time with the extension enabled, ensuring that the extension does not alter the default behavior. The problem is that unittest discover would then run the tests 3 times, once in their original location, once imported here, and once for the subclass. Therefore, we make use of unittest's load_tests Protocol which involves defining a load_tests function which returns an instance of a TestSuite class which explicitly sets which tests to run from that module.

Your error message suggests that PyTest does not like the TestSuite class our load_tests function returns. However, we are using an instance of a standard TestSuite class. I don't know enough about PyTest to know if it even can work with the load_tests protocol and/or TestSuite or if there is something special one would need to do to get it to work.

And, quite frankly, we don't need it to work with PyTest. That said if you can get it to work while at the same time keep things working as they are for us, then you are free to submit a PR. I'm willing to review a PR so long as the solution continues to use the standard lib only (PyTest is not in the standard lib so we are not interested in using it here). However, I am not interested in spending any more time on this than that.

waylan commented 7 months ago

Oh and by the way, this report suggests that this error was introduced in the latest release (3.5.2). However, this code has been in place since version 3.3 (released in 2020). If this just stopped working for you now, then the issue is elsewhere. Maybe you are using a newer version of PyTest which introduced a bug or dropped support or something???

kloczek commented 7 months ago

And, quite frankly, we don't need it to work with PyTest. That said if you can get it to work while at the same time keep things working as they are for us, then you are free to submit a PR. I'm willing to review a PR so long as the solution continues to use the standard lib only (PyTest is not in the standard lib so we are not interested in using it here). However, I am not interested in spending any more time on this than that.

That is OK however as I wrote "correctly written unittext based test suite can be correctly handled by pytest". Try to think as well that your module when it is integrated into whole distribution when it is packaged test suite execution is part not only your modules testing but any other package installed in build env as well. In other words such test suite has use case wider than only your needs ..

pytest is far more advanced and pluggable. This framework testing can alter testing process by simple install pytest extensions. Just in case .. I'm not trying to suggest switching to pytest. I'm only trying to tell that what you've wrote is 100% correct however this is not whole picture 😋

waylan commented 7 months ago

But our tests are fine. It is the load_tests protocol that Pytest is tripping on and our code matches almost exactly with the documented way to support the protocol. My assumption is that the problem is with PyTest's support (or lack thereof) of the load_tests protocol. Until you can prove to me that the problem is with our code, I'm not spending any more time on this.

facelessuser commented 7 months ago

I would suggest you open an issue over at Pytest regarding load_tests protocol and see what they say. While I can understand your desire to unify all the distributions you are managing with pytest, it is not explicitly necessary, and I don't think it is appropriate to suggest projects drop fully supported approaches to unittests just to unify pytest usage. Pytest advertises that it works with unittests, if this is not the case, they should fix that.

waylan commented 7 months ago

I just did a quick search and turned up pytest-dev/pytest#992 which acknowledges that Pytest does not support the load_tests protocol and has no plans to. Although, someone could write a plugin which does support it (maybe one exists, I didn't check). As the load_tests protocol is part of the standard feature set of the unittest lib, which we are using in its standard form, then this is completely on pytest, not us. I am closing this.