bot icon indicating copy to clipboard operation
bot copied to clipboard

Update to Python 3.10 and async-rediscache v1.0.0

Open ChrisLovering opened this issue 3 years ago • 3 comments

Description

This PR does the following things related to the PR description:

  1. Bumps the Python version up to 3.10, ensuring CI is updated too.
  2. Bumps the version of bot-core used to v8.0.0-beta.3 and pins transitive deps to exact versions
  3. Fixed breaking changes from the aioredis -> redis-py migration in async-rediscache v1.0.0-rc2

This PR also has the following kaizen commits:

  1. Mark the correct snekbox service as a dependency of the bot service
  2. Remove the, deprecated as of 3.10, call to get_event_loop() when there is no running loop
  3. Remove warnings in error handler tests
  4. Bump all dependencies to latest

ChrisLovering avatar Jul 23 '22 22:07 ChrisLovering

I don't understand the warnings fix here. setup is a coroutine function rather than a coroutine itself, so there should be no warnings from it just being imported, only if it is called and the coroutine returned not awaited (which doesn't seem to happen).

By importing the coro directly, the below warnings are raised (malloc enabled to see object allocation)

tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_already_handled
tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_check_failure
tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_command_invoke_error
tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_command_not_found_error_invoked_by_handler
tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_command_not_found_error_not_invoked_by_handler
tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_command_on_cooldown
  C:\Users\chris\src\bot\.venv\lib\site-packages\_pytest\python.py:776: RuntimeWarning: coroutine 'setup' was never awaited
    func(arg)

  Object allocated at:
    File "C:\Users\chris\src\bot\.venv\lib\site-packages\execnet\gateway_base.py", line 1237
      self.stack.append(as_bytes.decode("utf-8"))

I am not too sure why this happens either, as the coro is never called, but by namespacing the import like this the warnings go away.

Changing the cog to be shared across different tests in ErrorHandlerTests doesn't seem ideal, as some of the tests patch methods meaning they could affect each other. I cant see any obvious places it would actually make a difference here though,

This is fine with unnittest as the setUp function gets called before each test method.

ChrisLovering avatar Aug 03 '22 19:08 ChrisLovering

I don't understand the warnings fix here. setup is a coroutine function rather than a coroutine itself, so there should be no warnings from it just being imported, only if it is called and the coroutine returned not awaited (which doesn't seem to happen).

By importing the coro directly, the below warnings are raised (malloc enabled to see object allocation)

tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_already_handled
tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_check_failure
tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_command_invoke_error
tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_command_not_found_error_invoked_by_handler
tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_command_not_found_error_not_invoked_by_handler
tests/bot/exts/backend/test_error_handler.py::ErrorHandlerTests::test_error_handler_command_on_cooldown
  C:\Users\chris\src\bot\.venv\lib\site-packages\_pytest\python.py:776: RuntimeWarning: coroutine 'setup' was never awaited
    func(arg)

  Object allocated at:
    File "C:\Users\chris\src\bot\.venv\lib\site-packages\execnet\gateway_base.py", line 1237
      self.stack.append(as_bytes.decode("utf-8"))

I am not too sure why this happens either, as the coro is never called, but by namespacing the import like this the warnings go away.

I had a look into it and it seems to be fundamentally because of this: https://nose.readthedocs.io/en/latest/doc_tests/test_doctest_fixtures/doctest_fixtures.html?highlight=setup#module-level-fixtures

pytest sees a function called setup and tries to run it as a setup function for the tests, but doesn't await it hence the error.

The check for setup seems to be behind a check for the nose plugin in pytest (https://github.com/pytest-dev/pytest/blob/f43ddd8acd74a2c3242a3874420072836e0614aa/src/_pytest/python.py#L881), so the only thing I'm not sure about is why that would trigger since we don't use nose.

Anyway, maybe we should add a comment saying something like "setup should not be imported directly or pytest will try and run it" just to be clear?

Changing the cog to be shared across different tests in ErrorHandlerTests doesn't seem ideal, as some of the tests patch methods meaning they could affect each other. I cant see any obvious places it would actually make a difference here though,

This is fine with unnittest as the setUp function gets called before each test method.

Ooh yeah of course.

wookie184 avatar Aug 03 '22 20:08 wookie184

The check for setup seems to be behind a check for the nose plugin in pytest (https://github.com/pytest-dev/pytest/blob/f43ddd8acd74a2c3242a3874420072836e0614aa/src/_pytest/python.py#L881), so the only thing I'm not sure about is why that would trigger since we don't use nose.

You aren't quite looking at the right line: https://github.com/pytest-dev/pytest/blob/7.1.x/src/_pytest/python.py#L561-L568

This doesn't use nose, and it's what's called for us. Refer to TB:

    File "site-packages\_pytest\fixtures.py", line 1111
      result = call_fixture_func(fixturefunc, request, kwargs)
    File "site-packages\_pytest\fixtures.py", line 883
      fixture_result = next(generator)
    File "site-packages\_pytest\python.py", line 563
      _call_with_optional_argument(setup_module, request.module)
    File "site-packages\_pytest\python.py", line 776
      func(arg)

HassanAbouelela avatar Aug 03 '22 20:08 HassanAbouelela