pytest-dev / pytest-twisted

test twisted code with pytest
BSD 3-Clause "New" or "Revised" License
46 stars 24 forks source link

1.14.0: pytest fails in `testing/test_basic.py::test_async_fixture` unit #165

Closed kloczek closed 8 months ago

kloczek commented 8 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 + PYTHONPATH=/home/tkloczko/rpmbuild/BUILDROOT/python-pytest-twisted-1.14.0-2.fc36.x86_64/usr/lib64/python3.9/site-packages:/home/tkloczko/rpmbuild/BUILDROOT/python-pytest-twisted-1.14.0-2.fc36.x86_64/usr/lib/python3.9/site-packages + /usr/bin/pytest -ra -m 'not network' -q ============================= test session starts ============================== platform linux -- Python 3.9.18, pytest-8.1.1, pluggy-1.4.0 rootdir: /home/tkloczko/rpmbuild/BUILD/pytest-twisted-1.14.0 configfile: pytest.ini plugins: twisted-1.14.0, hypothesis-6.99.5 collected 60 items testing/test_basic.py .................F...................ss.ss........ [ 83%] .......... [100%] =================================== FAILURES =================================== ______________________________ test_async_fixture ______________________________ testdir = cmd_opts = ('/usr/bin/python3', '-m', 'pytest', '-v', '--reactor=default') @skip_if_no_async_await() def test_async_fixture(testdir, cmd_opts): pytest_ini_file = """ [pytest] markers = redgreenblue """ testdir.makefile('.ini', pytest=pytest_ini_file) test_file = """ from twisted.internet import reactor, defer import pytest import pytest_twisted @pytest_twisted.async_fixture( scope="function", params=["fs", "imap", "web"], ) @pytest.mark.redgreenblue async def foo(request): d1, d2 = defer.Deferred(), defer.Deferred() reactor.callLater(0.01, d1.callback, 1) reactor.callLater(0.02, d2.callback, request.param) await d1 return d2, @pytest_twisted.inlineCallbacks def test_succeed_blue(foo): x = yield foo[0] if x == "web": raise RuntimeError("baz") """ testdir.makepyfile(test_file) rr = testdir.run(*cmd_opts, timeout=timeout) > assert_outcomes(rr, {"passed": 2, "failed": 1}) /home/tkloczko/rpmbuild/BUILD/pytest-twisted-1.14.0/testing/test_basic.py:415: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ run_result = outcomes = {'failed': 1, 'passed': 2} def assert_outcomes(run_result, outcomes): formatted_output = format_run_result_output_for_assert(run_result) try: result_outcomes = run_result.parseoutcomes() except ValueError: assert False, formatted_output normalized_result_outcomes = { force_plural(name): outcome for name, outcome in result_outcomes.items() if name != "seconds" } > assert normalized_result_outcomes == outcomes, formatted_output E AssertionError: E ---- stdout E ============================= test session starts ============================== E platform linux -- Python 3.9.18, pytest-8.1.1, pluggy-1.4.0 -- /usr/bin/python3 E cachedir: .pytest_cache E hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase(PosixPath('/tmp/pytest-of-tkloczko/pytest-109/test_async_fixture0/.hypothesis/examples')) E rootdir: /tmp/pytest-of-tkloczko/pytest-109/test_async_fixture0 E configfile: pytest.ini E plugins: twisted-1.14.0, hypothesis-6.99.5 E collecting ... collected 3 items E E test_async_fixture.py::test_succeed_blue[fs] PASSED [ 33%] E test_async_fixture.py::test_succeed_blue[imap] PASSED [ 66%] E test_async_fixture.py::test_succeed_blue[web] FAILED [100%] E E =================================== FAILURES =================================== E ____________________________ test_succeed_blue[web] ____________________________ E E foo = (,) E E @pytest_twisted.inlineCallbacks E def test_succeed_blue(foo): E x = yield foo[0] E if x == "web": E > raise RuntimeError("baz") E E RuntimeError: baz E E test_async_fixture.py:21: RuntimeError E =============================== warnings summary =============================== E ../../../../usr/lib/python3.9/site-packages/_hypothesis_pytestplugin.py:444 E /usr/lib/python3.9/site-packages/_hypothesis_pytestplugin.py:444: PytestRemovedIn9Warning: Marks applied to fixtures have no effect E See docs: https://docs.pytest.org/en/stable/deprecations.html#applying-a-mark-to-a-fixture-function E return _orig_call(self, function) E E -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html E =========================== short test summary info ============================ E FAILED test_async_fixture.py::test_succeed_blue[web] - RuntimeError: baz E ==================== 1 failed, 2 passed, 1 warning in 0.75s ==================== E ---- stderr E E ---- E E assert {'failed': 1,...'warnings': 1} == {'failed': 1, 'passed': 2} E E Omitting 2 identical items, use -vv to show E Left contains 1 more item: E {'warnings': 1} E Use -v to get more diff /home/tkloczko/rpmbuild/BUILD/pytest-twisted-1.14.0/testing/test_basic.py:45: AssertionError ----------------------------- Captured stdout call ----------------------------- running: /usr/bin/python3 -m pytest -v --reactor=default in: /tmp/pytest-of-tkloczko/pytest-109/test_async_fixture0 ============================= test session starts ============================== platform linux -- Python 3.9.18, pytest-8.1.1, pluggy-1.4.0 -- /usr/bin/python3 cachedir: .pytest_cache hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase(PosixPath('/tmp/pytest-of-tkloczko/pytest-109/test_async_fixture0/.hypothesis/examples')) rootdir: /tmp/pytest-of-tkloczko/pytest-109/test_async_fixture0 configfile: pytest.ini plugins: twisted-1.14.0, hypothesis-6.99.5 collecting ... collected 3 items test_async_fixture.py::test_succeed_blue[fs] PASSED [ 33%] test_async_fixture.py::test_succeed_blue[imap] PASSED [ 66%] test_async_fixture.py::test_succeed_blue[web] FAILED [100%] =================================== FAILURES =================================== ____________________________ test_succeed_blue[web] ____________________________ foo = (,) @pytest_twisted.inlineCallbacks def test_succeed_blue(foo): x = yield foo[0] if x == "web": > raise RuntimeError("baz") E RuntimeError: baz test_async_fixture.py:21: RuntimeError =============================== warnings summary =============================== ../../../../usr/lib/python3.9/site-packages/_hypothesis_pytestplugin.py:444 /usr/lib/python3.9/site-packages/_hypothesis_pytestplugin.py:444: PytestRemovedIn9Warning: Marks applied to fixtures have no effect See docs: https://docs.pytest.org/en/stable/deprecations.html#applying-a-mark-to-a-fixture-function return _orig_call(self, function) -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html =========================== short test summary info ============================ FAILED test_async_fixture.py::test_succeed_blue[web] - RuntimeError: baz ==================== 1 failed, 2 passed, 1 warning in 0.75s ==================== =========================== short test summary info ============================ SKIPPED [2] testing/test_basic.py:77: reactor is default not qt5reactor SKIPPED [2] testing/test_basic.py:77: reactor is default not asyncio FAILED testing/test_basic.py::test_async_fixture - AssertionError: ============== 1 failed, 55 passed, 4 skipped in 74.00s (0:01:14) ============== ```
List of installed modules in build env: ```console Package Version ------------------ ----------- attrs 23.2.0 Automat 22.10.0 build 1.1.1 constantly 23.10.4 decorator 5.1.1 distro 1.9.0 dnf 4.19.0 exceptiongroup 1.1.3 gpg 1.23.2 greenlet 3.0.3 hyperlink 21.0.0 hypothesis 6.99.5 idna 3.6 importlib_metadata 7.0.1 incremental 22.10.0 iniconfig 2.0.0 installer 0.7.0 libdnf 0.73.0 packaging 24.0 pluggy 1.4.0 pyproject_hooks 1.0.0 pytest 8.1.1 python-dateutil 2.9.0.post0 setuptools 69.1.1 sortedcontainers 2.4.0 tokenize_rt 5.2.0 tomli 2.0.1 Twisted 24.3.0 typing_extensions 4.10.0 wheel 0.43.0 zipp 3.17.0 zope.event 5.0 zope.interface 6.2 ```

Please let me know if you need more details or want me to perform some diagnostics.

altendky commented 8 months ago

Hey, thanks for keeping this available as an RPM and for the issue report here. It looks like in https://github.com/pytest-dev/pytest-twisted/commit/3ac4209f8fb1486305b1223de16c61e7d261d677#diff-16608fad1d5f9586e8032d1202cf5d793becd18a6ae25d069e2615056d0361cdR192 I went and added @pytest.mark.redgreenblue to the fixture in that test. That is now triggering a deprecation warning due to https://github.com/pytest-dev/pytest/issues/3664 noted in the 8.0 changelog at https://docs.pytest.org/en/8.0.x/changelog.html#deprecations. Seems reasonable to just remove this mark. My best guess is that I wanted to make sure our decorator was able to apply over a mark. Maybe... I'm removing it in https://github.com/pytest-dev/pytest-twisted/pull/167. Then I get to try to remember how releases are setup here. If I don't have this done by tomorrow, feel free to ping me here.

Thanks again!

kloczek commented 8 months ago

One sec .. please allow me to check commit 😋

kloczek commented 8 months ago

Yep it looks like this PR fixes reported issue.

================================================================================== short test summary info ==================================================================================
SKIPPED [2] testing/test_basic.py:77: reactor is default not qt5reactor
SKIPPED [2] testing/test_basic.py:77: reactor is default not asyncio
========================================================================= 56 passed, 4 skipped in 76.00s (0:01:15) ==========================================================================

BTW .. looking on above output and https://pypi.org/project/asyncio/#history I think that keeping that optional test dependency of asyncio potentially could be removed as well because looks like this module is not maintained since 2015.

Thank you 😄 Feel free to close this ticket.

altendky commented 8 months ago

asyncio was integrated into the stdlib https://docs.python.org/3/library/asyncio.html. Did you see a place we actually specify a dependency on the asyncio project on PyPI? I looked quickly and didn't see one.

altendky commented 8 months ago

I'm trying to do a little improvement first, but here's the PR for a v1.14.1 version bump https://github.com/pytest-dev/pytest-twisted/pull/169. Hopefully I'll have it so that the GitHub release will trigger publishing to PyPI.

altendky commented 8 months ago

Alrighty, got it out. I think. https://pypi.org/project/pytest-twisted/1.14.1/ Let me know if you can build off that ok.

kloczek commented 8 months ago

Just found that my automation processed github release email notariatom and it was able build updated package and pytest passed without issues.

============================= test session starts ==============================
platform linux -- Python 3.9.18, pytest-8.1.1, pluggy-1.4.0
rootdir: /home/tkloczko/rpmbuild/BUILD/pytest-twisted-1.14.1
configfile: pytest.ini
plugins: twisted-1.14.1, hypothesis-6.99.6
collected 60 items

testing/test_basic.py .....................................ss.ss........ [ 83%]
..........                                                               [100%]

=========================== short test summary info ============================
SKIPPED [2] testing/test_basic.py:77: reactor is default not qt5reactor
SKIPPED [2] testing/test_basic.py:77: reactor is default not asyncio
=================== 56 passed, 4 skipped in 74.48s (0:01:14) ===================

Thank you 👍

Would you accept PR which drops python<=3.7 support? (3.7 was EOSed June last year) I've prepared such patch as I'm trying to cut tail of old python code as much as possible in my distro and I wold be happy to push those changes to your repo.

altendky commented 8 months ago

https://github.com/pytest-dev/pytest-twisted#python-2-support-plans

At some point it may become impractical to retain Python 2 support. Given the small size and very low amount of development it seems likely that this will not be a near term issue. While I personally have no need for Python 2 support I try to err on the side of being helpful so support will not be explicitly removed just to not have to think about it. If major issues are reported and neither myself nor the community have time to resolve them then options will be considered.

If supporting older versions is causing you trouble, I would consider that part of the cost of maintenance of this project even though you do that work outside of this repository. Feel free to open another ticket to discuss the hassles you run into due to the code supporting older versions.

Thanks for the report and continued repackaging of this project. Cheers.

kloczek commented 8 months ago

If supporting older versions is causing you trouble, I would consider that part of the cost of maintenance of this project even though you do that work outside of this repository. Feel free to open another ticket to discuss the hassles you run into due to the code supporting older versions.

No .. simple in my distro I'm trying at the moment update all possible python code to python 3.9 however I have as well separated patches with diffs dropping only python<=3.7 support. As I've tested all that already I'm trying to push at least those changes dropping python<=3.7 as much as I can to not maintain those patches in my build procedures 😋

In future when you will decide to drop older syntax just remember that you can do that using pyupgrade --py38 😄 That patch which I have 100% generated by that tool + another one with manual updates in setu.py

--- a/setup.py
+++ b/setup.py
@@ -13,7 +13,7 @@
     author_email="sda@fstab.net",
     url="https://github.com/pytest-dev/pytest-twisted",
     py_modules=["pytest_twisted"],
-    python_requires='>=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*',
+    python_requires='>=3.8',
     install_requires=["greenlet", "pytest>=2.3", "decorator"],
     extras_require={
         "dev": ["pre-commit", "black"],
@@ -30,13 +30,11 @@
         "Operating System :: OS Independent",
         "Programming Language :: Python",
         "Topic :: Software Development :: Testing",
-        "Programming Language :: Python :: 2",
-        "Programming Language :: Python :: 2.7",
-        "Programming Language :: Python :: 3",
-        "Programming Language :: Python :: 3.5",
-        "Programming Language :: Python :: 3.6",
-        "Programming Language :: Python :: 3.7",
         "Programming Language :: Python :: 3.8",
+        "Programming Language :: Python :: 3.9",
+        "Programming Language :: Python :: 3.10",
+        "Programming Language :: Python :: 3.11",
+        "Programming Language :: Python :: 3.12",
         "Programming Language :: Python :: Implementation :: CPython",
         "Programming Language :: Python :: Implementation :: PyPy",
     ],

Thanks for the report and continued repackaging of this project. Cheers.

Pleasure is on my side as well 👍 😄

altendky commented 8 months ago

Why do you need to actively remove the code when it also works up to 3.12?

kloczek commented 8 months ago

Why do you need to actively remove the code when it also works up to 3.12?

pyupgrade --py38-plus (mistake in typed param) only removes use older syntax/modules/etc (for example it removes use mock and six) and automatically upgrades for >=3.8.

altendky commented 8 months ago

Right. I've used pyupgrade but I'm not sure what benefit you would get if I were to remove Python 2.7-3.7 support. Given that the code works on 2.7-3.12 ~t~ it seems like there wouldn't be much cost to keeping it that way outside of any work I have to do in this repo. But I may well be missing something.

kloczek commented 8 months ago

FYI: I have already packaged +1.2k python modules as rpm packages. Some time ago I've filtered ALL python code over pyupgrade to update everything to +3.8. I have obligatory enabled use pytest as testing framwork. Wit some small patch I'm able to test modules without pytest/unittest test suites on scale

[tkloczko@pers-jacek SPECS]$ ls -1 python-*.spec |wc -l; grep -l ^%pytest python-*.spec |wc -l
1238
1220

In about 10% of all packages %pytest i temporary disabled because test suites are so trashed that pytest is not able to pass units scanning

[tkloczko@pers-jacek SPECS]$ grep -l ^%bcond_wit.*check python-*.spec |wc -l
124

In about 1/4 only some units are disabled by adde them to --deselect list

[tkloczko@pers-jacek SPECS]$ grep  ^%bcond_wit.*failing_tests python-*.spec | wc -l
300

All those issues and many more are more or less alredy know to maintainers

[tkloczko@pers-jacek SPECS]$ grep  "# BUG: pytest.*https://" python-*.spec | wc -l
400

BTW all already reported python modules issues .. 😋

[tkloczko@pers-jacek SPECS]$ grep  "# BUG:.*https://" python-*.spec | wc -l
934

Using that as platform I was able to prepare additional set of builds with installed additional pytest-benchmark in build env and alter %pytest macro to benchmark test suites execution on massive scale. All that without touching anything in rpm spec files..

Because I have all those pupgrade patches on rpm bcond which allows me easy produce results side by side with and without those patches and I've noticed that only occasionally in some handful cases it is some negative impact, (up to ~3%) and in really big number of cases some units are even 10% faster and in few cases even +20% faster. In most of the cases impast in neutral. All those tests are done with clearing/dropping all OS caches and reputing 3 times tests. Possible explanations of those observations are:

In other words: IMO that is enough often positive impact to go with such changes on 100% scale 😃

Now I'm in similar process of pushing all my packages to python +3.9 so as I've started that few days ago I've started pushing +3.8 changes as 3.8 has been EOSed Juna last year and I'm enough sure that all is stable (which is possible to see impact of that in my gh activity https://github.com/kloczek in recent days😋 )

Probably it will take me 3-4 weeks to finish review and testing all changes generated during two hours filtering all +4.8k packages. Only then I'll be able perform cumulative benchmarking but I'll be really surprised if that impact will be at least not as good as switching ALL python code to +3.8.

altendky commented 8 months ago

Thanks for the explanation. That's interesting that you are able to get these improvements.

kloczek commented 8 months ago

BTW .. I think that you should have look as well on ruff. This tool provides another dimension of some interesting tools to audit/improve python code.

altendky commented 8 months ago

I may do that at some point. Right now I'm struggling through the 'drop in replacement' uv. Mostly though, for me black and mypy are the key tools. Not saying there isn't value in other areas, or faster tools, but I've got so much general project cleanup to do that switching to ruff hasn't been a priority. Just getting similar ci setup across projects, and trying to figure out how to not copy paste it... is enough. (and yes i write actions and reusable workflows)

kloczek commented 8 months ago

between all those tools some functionalities are overlapping but some not. There is no single silver bullet tool in this case 😋