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

How to mark all coroutine tests with marker? #1793

Closed
purpleP opened this issue Aug 5, 2016 · 18 comments
Closed

How to mark all coroutine tests with marker? #1793

purpleP opened this issue Aug 5, 2016 · 18 comments
Labels
type: question general question, might be closed after 2 weeks of inactivity

Comments

@purpleP
Copy link

purpleP commented Aug 5, 2016

There is a plugin[https://pypi.python.org/pypi/pytest-asyncio] for testing coroutines that defines custom marker that basically takes coroutine function and runs in in loop until it's compelted.

I'd like to automate the process of marking these coroutines tests.
The best I could do after couple of hours of trying, reading docs and your code is this.

def pytest_pycollect_makeitem(collector, name, obj):
    if _is_coroutine(obj):
        wrappped = pytest.mark.asyncio(obj)
        return pytest.Function(name=name, parent=collector)

Which seems to be working if there isn't parametrized marker in test.
What I need to do is very simple as you can see. All I need is to use the function that you using to create test items, so that this item would be just as if pytest found it itself. But I can't find this function in your code.

  @pytest.mark.parametrize(
      'args,expected',
      (
          ((), [()]),
          (['ab'], [('a',), ('b',)]),
          (
              [range(2), range(3)],
              [(0, 0), (0, 1), (0, 2), (1, 0), (1, 1), (1, 2)],
          ),
          ([range(0), range(2), range(3)], []),
          ([range(2), range(0), range(3)], []),
          ([range(2), range(3), range(0)], []),
      ),
  )
  async def test_product_zero_iterables(args, expected):
      """Check that product returns empty tuple when there are no iterables."""
      assert (await aitertools.alist(
          aitertools.product(*args)
      )) == expected
        fixture 'args' not found
        available fixtures: monkeypatch, tmpdir, tmpdir_factory, pytestconfig, unused_tcp_port_factory, cov, record_xml_property, capsys, event_loop, recwarn, capfd, event_loop_process_pool, cache, unused_tcp_port
        use 'py.test --fixtures [testpath]' for help on them.

@nicoddemus
Copy link
Member

You can set a variable named pytestmark in your module to apply that mark to all tests on it:

pytestmark = pytest.mark.asyncio

@nicoddemus nicoddemus added the type: question general question, might be closed after 2 weeks of inactivity label Aug 5, 2016
@purpleP
Copy link
Author

purpleP commented Aug 6, 2016

@nicoddemus That could work in my particular case right now, but not in general, as not all tests could be async. I think I was on the right track, I just need a little help figuring out how to create test item the way it's created by pytest itself (the best way to do that of course is just use the same function).

PS. Okay this doesn't even work, because it seems that pytest tries to apply mark after collecting items.

@nicoddemus
Copy link
Member

In that case I think this should work (untested):

# conftest.py
def pytest_collection_modifyitems(items):
    for item in items:
        if _is_coroutine(item.function):
            item.add_marker('asyncio')

@purpleP
Copy link
Author

purpleP commented Aug 7, 2016

@nicoddemus Nope, That's the first thing I've tried and it has the same problem. It's happening after the collection started and results in error. What's happening I think is pytest sees this async def functions as generators and tries to iterate over them, but It can't because they aren't iterables. That's why I keep telling you - I've found the hook I want and it almost working.

@RonnyPfannschmidt
Copy link
Member

since the plugin is under pytest-dev, we should probably just fix it directly to take async functions instead of having users help themselfes with local hacks

as far as i can tell pytest-asyncio is in error, since it should check for coroutines/async functions instead of markers anyway

@nicoddemus
Copy link
Member

@purpleP oh ok, sorry about that.

@RonnyPfannschmidt that's a nice idea... is it easy/safe to check if a test function is a coroutine?

@RonnyPfannschmidt
Copy link
Member

async functiosn have a new flag and the inspect module has a test function

idiomatic pre async def coroutines are marked with the coroutine decorator from the stdlib

@purpleP
Copy link
Author

purpleP commented Aug 8, 2016

@nicoddemus @RonnyPfannschmidt There isinspect.iscoroutinefunction and asyncio.iscoroutine. I think it's better to use asyncio because it returns True for both async def coroutines and asyncio coroutines, while inspect.iscoroutinefunction return True only for async def coroutines.

And just in case - I do understand that if case a test is coroutine pytest should create a loop and run the test in it?

Another question is when this will be ready to use? Because I need the code to work ASAP. If this would take a week for example I'd like to still like to have an answer to my question. I mean how to that with hooks.

@RonnyPfannschmidt
Copy link
Member

i cant make an eta, im on a vacation atm ^^ - for you the most simple solution might be to make a pr with a fix against pytest-asyncio, then we can merge and the pypi owner can release later,

@Tinche
Copy link
Member

Tinche commented Aug 8, 2016

I'm the main maintainer of pytest-asyncio. Not sure I like the idea of automatically treating all coroutines as asyncio tests, without the marker.

  1. There are other frameworks using coroutines (Twisted, Tornado, Curio), this would make it impossible for example pytest-curio to be installed alongside pytest-asyncio
  2. the marker can take parameters (just one currently, more later maybe) so it's useful for it to be present

@purpleP
Copy link
Author

purpleP commented Aug 8, 2016

@Tinche No one is saying marker isn't useful. And not all coroutines should be automatically marked with it, only ones that match purest test discovery pattern. I don't see how it makes other asynchronous frameworks unable to work with Asuncion etc. Could you provide example?

@purpleP purpleP closed this as completed Aug 8, 2016
@purpleP purpleP reopened this Aug 8, 2016
@RonnyPfannschmidt
Copy link
Member

@Tinche the idea is to use async def or @asyncio.coroutine AS indicators in Addition to the marker,

@Tinche
Copy link
Member

Tinche commented Aug 9, 2016

@pytest.mark.asyncio
async def test_x():
    # test something

@pytest.mark.curio
async def test_y():
    # test something else

If we automatically treat all coroutine test functions as asyncio, this won't work? Maybe I'm misunderstanding something.

@purpleP
Copy link
Author

purpleP commented Aug 9, 2016

@Tinche Yeah, that's most likely wouldn't work. I get it now. But still I think that that's a reasonable default.
My opinion on this is that automatic marking should be a part of your library and work via pytest hooks. Something like constant in test module could be used to give default mark and all other async tests that uses different coroutine libraries can be marked explicitly. This is totally doable and totally reasonable.

And that brings us to square one. Can anyone finally say to me what function should used by default to create test items?

@RonnyPfannschmidt
Copy link
Member

@Tinche i think it totally makes sense to convenicence

if people really do have mixed codebases, they should be enabled to disable pytest-asyncio for folders based on a conftest and/or pick different defaults

as a user my basic expectation would be - if you use pytest-asyncio, you use asyncio, and trus async tests do use the eventloop of asyncio

and in order to maintain minimal api i think opt-out instead of opt-in seems fair

take this with a grain of salt since its a personal oppinion

@Tinche
Copy link
Member

Tinche commented Aug 9, 2016

I'm going to respectfully disagree here. pytest-asyncio simply being installed making coroutines in tests unusable for any other purpose is not reasonable in my opinion. We're introducing a whole lot of magic that will need additional complexity to opt out of because it's hard to mark your functions?

Maybe we could compromise. If your test file contains the word 'asyncio' (so you can call your test file test_asyncio_blabla.py), then we could automatically treat all coroutines inside as being marked with asyncio. How about this approach?

@purpleP
Copy link
Author

purpleP commented Aug 9, 2016

@Tinche
@RonnyPfannschmidt
First of all because pytest already has pytestmark = ... if we fix pytest treating coroutines as generators anybody who uses only one async framework is basically covered.

Second for users that use multiple async frameworks the approach I've described above would also work.
Now to @Tinche argument

  • I think that adding one line like default_async_mark = pytest.mark.asyncio to test module or conftest isn't

a whole lot of magic

  • Additional complexity? Yes, but not much. It basically comes down to one little hook. If the pytest developer would think the way you think pytest wouldn't exists and everybody would still use unittest. No magic, not a whole lot of complexity, a lot of code for users to write that doesn't have anything with test logic, you know. We're writing code for others to use, we have responsibility to save their time, not ours.

@nicoddemus
Copy link
Member

We're introducing a whole lot of magic that will need additional complexity to opt out of because it's hard to mark your functions?

I agree with you here... marking the test functions should be simple enough, once you know the appropriate tricks (like pytestmark). Perhaps that could be added to the documentation so users are more aware of that approach.

Maybe we could compromise. If your test file contains the word 'asyncio' (so you can call your test file test_asyncio_blabla.py), then we could automatically treat all coroutines inside as being marked with asyncio. How about this approach?

If that route is taken, I would suggest to only check for test_* functions if they are a coroutine, and this could be opted in by using a configuration variable in pytest.ini like async_auto_mark=1 or something (defaulting to False).

@nicoddemus nicoddemus added plugin: junitxml related to the junitxml builtin plugin and removed plugin: junitxml related to the junitxml builtin plugin labels Feb 2, 2017
@nicoddemus nicoddemus removed the plugin: junitxml related to the junitxml builtin plugin label Apr 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question general question, might be closed after 2 weeks of inactivity
Projects
None yet
Development

No branches or pull requests

4 participants