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

Mixing pytest tests and twisted.trial.unitest.TestCase results in exception when running with asyncio reactor #147

Open
pawelmhm opened this issue Jan 3, 2022 · 5 comments

Comments

@pawelmhm
Copy link

pawelmhm commented Jan 3, 2022

I have a project with tests inheriting from both twisted.trial.unittest.TestCase and tests not inheriting from anything, just plain Python objects. When running without asyncio reactor, they all work fine. When I pass --reactor flag to pytest via pytest-twisted they are failing with "RuntimeError: This event loop is already running".

My expectation is that both trial and non-trial tests will work, because why not? If they worked with default reactor why not work with asyncio reactor.

Steps to reproduce. Take following file:

# test_reactor.py
from twisted.trial.unittest import TestCase


class TestCrawlCase: 
    def test_if_parametrize_works(self):
        assert 1 == 1

    def test_another(self):
        assert 1 == 1


class AnotherTest(TestCase):
    def test_another_test_case(self):
        assert 2 == 2

Now run this file without any arguments:

ython -m pytest test_reactor.py -vsx
============================================================================================================ test session starts =============================================================================================================
platform linux -- Python 3.9.4, pytest-6.2.3, py-1.11.0, pluggy-0.13.1 -- /home/.../bin/python
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/home/.../.hypothesis/examples')
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /home/pawel/Documents/projects/Spiders, configfile: pytest.ini
plugins: Faker-8.1.3, hypothesis-6.10.1, benchmark-3.4.1, twisted-1.13.4
collected 3 items                                                                                                                                                                                                                            

test_reactor.py::TestCrawlCase::test_if_parametrize_works PASSED
test_reactor.py::TestCrawlCase::test_another PASSED
test_reactor.py::AnotherTest::test_another_test_case PASSED

Now run same file with --reactor=asyncio

 python -m pytest test_reactor.py -vsx --reactor=asyncio
============================================================================================================ test session starts =============================================================================================================
platform linux -- Python 3.9.4, pytest-6.2.3, py-1.11.0, pluggy-0.13.1 -- /home/pawel/.pyenv/versions/../bin/python
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/home...')
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /home/.../directory, configfile: pytest.ini
plugins: Faker-8.1.3, hypothesis-6.10.1, benchmark-3.4.1, twisted-1.13.4
collected 3 items                                                                                                                                                                                                                            

test_reactor.py::TestCrawlCase::test_if_parametrize_works PASSED
test_reactor.py::TestCrawlCase::test_another PASSED
test_reactor.py::AnotherTest::test_another_test_case FAILED
test_reactor.py::AnotherTest::test_another_test_case ERROR

This fails with:

=================================================================================================================== ERRORS ===================================================================================================================
__________________________________________________________________________________________ ERROR at teardown of AnotherTest.test_another_test_case ___________________________________________________________________________________________

self = <test_reactor.AnotherTest testMethod=test_another_test_case>, result = <TestCaseFunction test_another_test_case>

    def _classCleanUp(self, result):
        try:
>           util._Janitor(self, result).postClassCleanup()


self = <_UnixSelectorEventLoop running=True closed=False debug=False>

    def _check_running(self):
        if self.is_running():
>           raise RuntimeError('This event loop is already running')
E           RuntimeError: This event loop is already running

../../../.pyenv/versions/3.9.4/lib/python3.9/asyncio/base_events.py:578: RuntimeError

@pawelmhm pawelmhm changed the title Mixin pytest tests and twisted.trial.unitest.TestCase results in exception when running with asyncio reactor Mixing pytest tests and twisted.trial.unitest.TestCase results in exception when running with asyncio reactor Jan 3, 2022
@altendky
Copy link
Member

altendky commented Jan 4, 2022

Maybe we can handle this but I am a bit hesitant to get into trying to support trial tests. Seems like it could be a big can of worms. Which thing is supposed to handle the test methods? Anyways, I'll leave this somewhere on my to-do list to at least understand a bit better what's up.

@pawelmhm
Copy link
Author

pawelmhm commented Jan 5, 2022

Maybe we can handle this but I am a bit hesitant to get into trying to support trial tests

I somehow assumed twisted trial tests are supported. Maybe some misunderstanding from my side. There appears to be no info that they are not supported and trial is part of twisted. So, if pytest-twisted says it "allows to test code, which uses the twisted framework" I thought this includes trial too. When I have only trial tests pytest-twisted seems to works fine. You can try to inherit from TestCase in plain object in my sample and you'll see it works fine. I would say in practice it actually does support trial, it just does not support mixing trial and non-trial when specifying asyncio reactor.

Until now, I was running them with pytest without pytest-twisted. It worked fine. But then I wanted to switch to asyncio reactor. This is why I reached pytest-twisted. It allows me to specify reactor.

Browsing code I stumbled on this part: https://github.com/pytest-dev/pytest-twisted/blob/main/pytest_twisted.py#L365 which suggests at the moment pytest-twisted supports other tests? It says: "because our interface allowed people to return deferreds
from arbitrary tests so we kinda have to keep this up for now". Is twisted trial included in this "arbitrary tests" definition?

If you decide not to support twisted-trial I can just change my testing code, but I would like to get clarity from you and maybe some suggestion what is the best way to do it if I stick to pytest-twisted. Maybe I can just use other tools and refactor my code, this is possible too.

Probably mixing twisted trial and non twisted trial is not a great idea, but someone in my codebase started to do it in the past. There are some reasons for it. Twisted trial did not allow to easy parametrize tests, but trial was used for inlinecallbacks tests. It worked ok so no one saw a problem.

I see the problem here is that twisted trial does not run non trial test cases, does not run them if I do

> python -m twisted.trial --reactor=asyncio test_reactor

Twisted Trial runs just one tests, it ignores non trial test. Which is expected. Pytest runs all three and with pytest-twisted when I add traceback.print_stack() to lib/python3.9/asyncio/base_events.py:589: in run_forever I see run_forever() is called even for non async test that does not inherit from twisted.trial and does not use any deferreds.

test_reactor.py::TestCrawlCase::test_if_parametrize_works 


\ someone calling run forever 



  File "/home/.../projects/twisted/src/twisted/internet/asyncioreactor.py", line 255, in run
    self._asyncioEventloop.run_forever()
  File "/home/.../.pyenv/versions/3.9.4/lib/python3.9/asyncio/base_events.py", line 588, in run_forever
    traceback.print_stack()
PASSED

and then twisted.trial async test calls asyncio run_forever and fails on this line, when doing clean up for pending calls https://github.com/twisted/twisted/blob/trunk/src/twisted/trial/util.py#L128

@pawelmhm
Copy link
Author

pawelmhm commented Jan 5, 2022

just replacing trial with pytest-twisted does the trick and works fine, so I could just remove all twisted trial and replace them with pytest_twisted.inlineCallbacks if you decide not to support trial.

from twisted.trial.unittest import TestCase
import os
from twisted.internet import threads
import pytest_twisted


class TestCrawlCase: #(TestCase):
    def test_if_parametrize_works(self):
        pass
    
    def test_if_correct_reactor(self):
        from twisted.internet import reactor
        from twisted.internet.asyncioreactor import AsyncioSelectorReactor
        assert isinstance(reactor, AsyncioSelectorReactor)



class TestAnotherTest: #(TestCase):
    @pytest_twisted.inlineCallbacks
    def test_some_stuff(self):
        res = yield threads.deferToThread(os.listdir, ".")
        assert len(res) > 0
    
    def test_another_test_case(self):
        pass

@altendky
Copy link
Member

altendky commented Jan 5, 2022

From my limited experience trial is there to be a test runner that supports async tests with the twisted framework. pytest-twisted is here to let you do that same thing but built on pytest. They each handle managing a reactor and getting it to process the tests. I don't know my way around trial to know how we would mix with them, how much we can leave to trial to handle, and how much pytest-twisted would need to implement to fill in the gaps.

Side note, you may have to use inline callbacks for other reasons, but note that pytest-twisted supports async/await as well.

Thanks for sharing your research here. If I find some time I'll see what I can make of all this.

@Ignalion
Copy link

I think this issue is related to new reactor for each test, like pytest-asyncio because asyncio reactor uses run_forever() which is explicitly checking that the reactor isn't running in the following function.

    def iterate(self, timeout):
        self._asyncioEventloop.call_later(timeout + 0.01,
                                          self._asyncioEventloop.stop)
        self._asyncioEventloop.run_forever()

So for mixing twisted trial with pytest-twisted we would need to isolate reactor runs for that case.

There is actually the second issue, after twisted trial test, pytest-twisted would raise another exception that reactor is actually stopped because twisted greenlet is dead. I think the reason for that is that call_later call in the iterate method that actually stops the asyncioloop but keeps the reactor in weird 'running' state.

It would be actually great to add check whether reactor is already running to the twisted trial but this is almost impossible since all the twisted code doesn't leave the possibility to using it somewhere outside the twisted.

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

No branches or pull requests

3 participants