test: use anyio instead of pytest-asyncio

[dlax]

Following comments from #146 (comment), this is in preparation for #29 via #146.

---

This is in preparation for adding support for other async libraries,
through anyio. AnyIO pytest plugin is used in replacement for
pytest-asyncio:

• either the pytest.mark.asyncio is replaced by pytest.mark.anyio, or,
• we rely on the 'anyio_backend' fixture that is pulled in 'aconn_cls'
  fixture (and hence 'aconn') providing automatic detection for test
  functions using it.

The 'anyio_backend' fixture is parametrized to only use asyncio and
selects the event loop policy we need on Windows platform as previously
done in pytest_sessionstart(), but only for Python version 3.8 or
higher.

This fixture is defined in main conftest.py, as well as in
pool/conftest.py since we'll change the former to support more async
backend while keeping the later asyncio-only for now.

Some tests use asyncio explicitly so they would not run with another
backend; accordingly, we mark them with
@pytest.mark.parametrize('anyio_backend', ['asyncio']).

Function test_concurrency_async.py::test_ctrl_c is no longer 'async'
because its code does not directly use asyncio (it's done through a
subprocess); but the 'async def' was needed before in order for
pytest-asyncio to run it since the test module had a global
pytest.mark.asyncio (and we were using the "auto" mode).

[dvarrazzo]

I am not sure that pytest copes well with having more than one conftest.py defined into a single root. If it's not complaining, I suspect it's just ignoring this file.

Do we need a different conftest for pool? Is this becoming a sort of separated test suite? If so, I think it should be moved as sibling (tests/pool -> tests_pool) to make sure that pytest finds a single conftest. But then the tests should be run in two commands... I'm not sure that's right.

[dlax]

Yes, pytest handles this as expected: the anyio_backend fixture defined here will override the one defined in outer conftest.py. See https://docs.pytest.org/en/latest/how-to/fixtures.html#override-a-fixture-on-a-folder-conftest-level

In fact, that's temporary as this file would go away when we'll port pool code to use anyio.

[dvarrazzo]

This should do too, with the right imports:

Suggested change

	asyncio_options = {} # type: ignore[var-annotated]
	asyncio_options: Dict[str, Any] = {}

[dvarrazzo]

Uhm... but mostly, why do we need a global asyncio_options module-level dictionary at all here and don't we do everything in the fixture itself?

[dlax]

The global asyncio_options is imported from elsewhere (from tests/pool/conftest.py in this PR, and from test_waiting.py, at least temporarily in 89bdf08#diff-ec162367f2b494baea5f25f3d81908ca9edbbae2b6f129fd2b3bb3c89d626a20).

[dvarrazzo]

I don't get exactly what is the reason for specifying this parametrize here. I understand that anyio_backend is a parametrized fixture that yields exactly one backend right now, right?

I understand that your plan is to eventually change the fixture and make it yield ['asyncio', 'trio'], but that a test like this cannot run as it is on trio because it uses explicitly asyncio.gather(). That might be fine, but I think that pre-emptively skipping this test on trio might hide failures. I would rather not specify a @parametrize for this test, look at everything failing on trio, and only then write a matching test test_concurrent_execution_trio... Although that wouldn't scale in the direction of having different backends.

What do you think will be supported in the future?

Also, alternative implementation, to make tests more readable (although it would add the same shortcoming of hiding tests when a new backend is introduced): how about dropping the module-level import asyncio, and having a fixture called asyncio, which would yield the asyncio module if we are testing asyncio and would skip the tests if we are testing trio? Same for other backends.

Suggested change

	@pytest.mark.parametrize("anyio_backend", ["asyncio"])
	async def test_concurrent_execution(aconn_cls, dsn, anyio_backend):
	async def test_concurrent_execution(aconn_cls, dsn, asyncio):

[dlax]

Indeed, this mark is somehow a left-over which could be dropped. Namely, I initially did not port those tests (the ones using asyncio explicitly) and thus needed this mark; but ultimately, I decided to port them in f43c709#diff-b3b8cfbbb6a3720571b3ceceb3dc998d151667f62e5738a225bd0234cbde51ff (done through if anyio_backend_name == "asyncio":, though we might arguably find a nicer way).

[dhirschfeld]

Looks good barring what appears to be a network issue:

[saolof]

This was just marked as ready for review? I skimmed through it and it looked good to me, though someone more deeply familiar with the actual psycopg codebase may be better qualified to review this.

[dvarrazzo]

Merged, thank you! I'll try to cherry-pick this on maint 3.1 too, otherwise it will be easy to cause conflicts.

[dvarrazzo]

This change seems to have caused some instability in the tests on windows:

• https://github.com/psycopg/psycopg/actions/runs/4096339028/jobs/7063905786
• https://github.com/psycopg/psycopg/actions/runs/4091876834/attempts/1

we should figure out what is going on there... :\

[dlax]

we should figure out what is going on there... :\

I think this is because there used to be an unconditional asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy()) which was replaced by the asyncio_options dict in the assumption that no asyncio loop would be instantiated outside the anyio fixture, which is wrong in tests/pool/test_pool_async_noasyncio.py.

I'll work on a fix; thank you for letting me know.