Fix warnings and resource usage problems in asyncio unittests #2258

kristjanvalur · 2022-06-28T10:33:05Z

Pull Request check-list

Please make sure to review and check all of these items:

Does $ tox pass with this change (including linting)?
Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
Is the new or changed code fully tested?
Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
Is there an example added to the examples folder (if applicable)?
Was the change added to CHANGES file?

Description of change

pytest-asyncio auto mode is recommended unless there is a need to run asyncio tests using different async frameworks.
Using asyncio and removing the overly generous pytestmark reduces warning noise in the testsuite. (Python 3.6 still needs it since the 'auto' mode isn't supported for it)

Perform various cleanup of asyncio connections within the fixture framework of pytest-asyncio, thus avoiding having
to run them via the general pytest non-async cleanup mechanism.

Continued work on making unittests more robust so that we don't get sporadic test failures for unrelateds PRs

codecov-commenter · 2022-06-28T15:20:30Z

Codecov Report

Merging #2258 (8080893) into master (e6cd4fd) will decrease coverage by 0.23%.
The diff coverage is 85.49%.

@@            Coverage Diff             @@
##           master    #2258      +/-   ##
==========================================
- Coverage   91.85%   91.62%   -0.24%     
==========================================
  Files         108      108              
  Lines       27783    27821      +38     
==========================================
- Hits        25521    25490      -31     
- Misses       2262     2331      +69

Impacted Files	Coverage Δ
tests/test_asyncio/test_cluster.py	`97.30% <0.00%> (-0.14%)`	⬇️
tests/test_asyncio/test_encoding.py	`96.77% <0.00%> (-2.16%)`	⬇️
tests/test_asyncio/test_json.py	`99.75% <ø> (-0.25%)`	⬇️
tests/test_asyncio/test_lock.py	`98.46% <0.00%> (-1.54%)`	⬇️
tests/test_asyncio/test_scripting.py	`98.13% <0.00%> (-1.87%)`	⬇️
tests/test_asyncio/test_sentinel.py	`97.97% <0.00%> (-1.36%)`	⬇️
tests/test_asyncio/conftest.py	`81.37% <62.06%> (-12.01%)`	⬇️
tests/test_asyncio/test_connection.py	`95.00% <66.66%> (-2.37%)`	⬇️
tests/test_asyncio/test_monitor.py	`89.47% <66.66%> (-1.44%)`	⬇️
tests/test_asyncio/test_pipeline.py	`99.63% <66.66%> (-0.37%)`	⬇️
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6cd4fd...8080893. Read the comment docs.

dvora-h · 2022-07-25T08:35:23Z

@kristjanvalur Can you please resolve the conflicts with master?

kristjanvalur · 2022-07-25T10:15:22Z

Right. Btw, seems like almost every PR I submit needs to have the unit-tests tweaked to avoid spurious failures :)

Remove overly genereric `pytestmark=pytest.mark.asyncio` causing lots of warning noise

this fixture is now async, avoiding teardown problems with missing event loops.

This fixes a resource usage warning in the async unittests.

Use a simple fixture where possible, otherwise manually call pubsub.close()

…it tests

kristjanvalur · 2022-07-26T16:56:09Z

Cheers!

…2258) * Use pytest-asyncio in auto mode Remove overly genereric `pytestmark=pytest.mark.asyncio` causing lots of warning noise * Use "Factories as Fixtures" test pattern for the `create_redis` fixture this fixture is now async, avoiding teardown problems with missing event loops. * Fix sporadic error on fast event loops, such as `--uvloop` * Close connection, even if "username" was in kwargs This fixes a resource usage warning in the async unittests. * Do async cleanup of acl passwords via a fixture * Remove unused import, fix whitespace * Fix test with missing "await" * Close pubsub objects after use in unittest Use a simple fixture where possible, otherwise manually call pubsub.close() * re-introduce `pytestmark=pytest.mark.asyncio` for python 3.6 * Use context manager to clean up connections in connection pool for unit tests * Provide asynccontextmanager for python 3.6 * make `test_late_subscribe()` more robuste * Catch a couple of additional leaked resources

* Add support for async graph * linters * fix docstring * Use retry mechanism in async version of Connection objects (#2271) * fix is_connected (#2278) * fix: workaround asyncio bug on connection reset by peer (#2259) Fixes #2237 * Fix crash: key expire while search (#2270) * fix expire while search * sleep * docs: Fix a few typos (#2274) * docs: Fix a few typos There are small typos in: - redis/cluster.py - redis/commands/core.py - redis/ocsp.py - tests/test_cluster.py Fixes: - Should read `validity` rather than `valididy`. - Should read `reinitialize` rather than `reinitilize`. - Should read `farthest` rather than `farest`. - Should read `commands` rather than `comamnds`. * Update core.py * async_cluster: fix concurrent pipeline (#2280) - each pipeline should create separate stacks for each node * Add support for TIMESERIES 1.8 (#2296) * Add support for timeseries 1.8 * fix info * linters * linters * fix info test * type hints * linters * Remove verbose logging from `redis-py/redis/cluster.py` (#2238) * removed the logging module and its corresponding methods * updated CHANGES * except block for RedisClusterException and BusyLoadingError removed * removed unused import (redis.exceptions.BusyLoadingError) * empty commit to re-trigger Actions workflow * replaced BaseException with Exception * empty commit to re-trigger Actions workflow * empty commit to re-trigger Actions workflow * redundant logic removed * re-trigger pipeline * reverted changes * re-trigger pipeline * except logic changed * redis stream example (#2269) * redis stream example * redis stream example on docs/examples.rst Co-authored-by: pedro.frazao <perl.pf@netcf.org> * Fix: `start_id` type for `XAUTOCLAIM` (#2257) * Changed start_id type for xautoclaim * Added to changes Co-authored-by: dvora-h <67596500+dvora-h@users.noreply.github.com> * Doc add timeseries example (#2267) * DOC add timeseries example * DOC add timeseries examples * Apply suggestions * Fix typo Detention period => Retention period Co-authored-by: Gauthier Imbert <gauthier@PC17> * Fix warnings and resource usage problems in asyncio unittests (#2258) * Use pytest-asyncio in auto mode Remove overly genereric `pytestmark=pytest.mark.asyncio` causing lots of warning noise * Use "Factories as Fixtures" test pattern for the `create_redis` fixture this fixture is now async, avoiding teardown problems with missing event loops. * Fix sporadic error on fast event loops, such as `--uvloop` * Close connection, even if "username" was in kwargs This fixes a resource usage warning in the async unittests. * Do async cleanup of acl passwords via a fixture * Remove unused import, fix whitespace * Fix test with missing "await" * Close pubsub objects after use in unittest Use a simple fixture where possible, otherwise manually call pubsub.close() * re-introduce `pytestmark=pytest.mark.asyncio` for python 3.6 * Use context manager to clean up connections in connection pool for unit tests * Provide asynccontextmanager for python 3.6 * make `test_late_subscribe()` more robuste * Catch a couple of additional leaked resources * Graph - add counters for removed labels and properties (#2292) * grpah - add counters for removed labels and properties * added mock graph result set statistics * docstrings for graph result set statistics * format * isort * moved docstrings into functions * cleaning up the readme and moving docs into readthedocs (#2291) * cleaning up the readme and moving docs into readthedocs * examples at the end as per pr comments * async_cluster: fix max_connections/ssl & improve args (#2217) * async_cluster: fix max_connections/ssl & improve args - set proper connection_class if ssl = True - pass max_connections/connection_class to ClusterNode - recreate startup_nodes to properly initialize - pass parser_class to Connection instead of changing it in on_connect - only pass redis_connect_func if read_from_replicas = True - add connection_error_retry_attempts parameter - skip is_connected check in acquire_connection as it is already checked in send_packed_command BREAKING: - RedisCluster args except host & port are kw-only now - RedisCluster will no longer accept unknown arguments - RedisCluster will no longer accept url as an argument. Use RedisCluster.from_url - RedisCluster.require_full_coverage defaults to True - ClusterNode args except host, port, & server_type are kw-only now * async_cluster: remove kw-only requirement from client Co-authored-by: dvora-h <67596500+dvora-h@users.noreply.github.com> * fix review comments * fix * fix review comments * fix review comments Co-authored-by: Chayim <chayim@users.noreply.github.com> Co-authored-by: szumka <106675199+szumka@users.noreply.github.com> Co-authored-by: Mehdi ABAAKOUK <sileht@sileht.net> Co-authored-by: Tim Gates <tim.gates@iress.com> Co-authored-by: Utkarsh Gupta <utkarshgupta137@gmail.com> Co-authored-by: Nial Daly <34862917+nialdaly@users.noreply.github.com> Co-authored-by: pedrofrazao <603718+pedrofrazao@users.noreply.github.com> Co-authored-by: pedro.frazao <perl.pf@netcf.org> Co-authored-by: Антон Безденежных <gamer392@yandex.ru> Co-authored-by: Iglesys <g.imbert34@gmail.com> Co-authored-by: Gauthier Imbert <gauthier@PC17> Co-authored-by: Kristján Valur Jónsson <sweskman@gmail.com> Co-authored-by: DvirDukhan <dvir@redis.com>

kristjanvalur force-pushed the kristjan/unittest-warning-cleanup branch 2 times, most recently from 7b9dedc to ebcd7d1 Compare June 28, 2022 11:27

kristjanvalur marked this pull request as draft June 28, 2022 11:35

kristjanvalur changed the title ~~Use pytest-asyncio in auto mode~~ Fix warnings and resource usage problems in asyncio unittests Jun 28, 2022

kristjanvalur force-pushed the kristjan/unittest-warning-cleanup branch 5 times, most recently from 2cd2bc1 to 423c0e6 Compare June 28, 2022 16:53

kristjanvalur marked this pull request as ready for review June 29, 2022 16:35

kristjanvalur added 13 commits July 25, 2022 10:16

Use pytest-asyncio in auto mode

3b01dbd

Remove overly genereric `pytestmark=pytest.mark.asyncio` causing lots of warning noise

Use "Factories as Fixtures" test pattern for the create_redis fixture

451ac03

this fixture is now async, avoiding teardown problems with missing event loops.

Fix sporadic error on fast event loops, such as --uvloop

1a1c0d7

Close connection, even if "username" was in kwargs

56bc4a1

This fixes a resource usage warning in the async unittests.

Do async cleanup of acl passwords via a fixture

8d26c91

Remove unused import, fix whitespace

ea68f78

Fix test with missing "await"

c745185

Close pubsub objects after use in unittest

a69d73a

Use a simple fixture where possible, otherwise manually call pubsub.close()

re-introduce pytestmark=pytest.mark.asyncio for python 3.6

f2c100c

Use context manager to clean up connections in connection pool for un…

d7ca47f

…it tests

Provide asynccontextmanager for python 3.6

011b76b

make test_late_subscribe() more robuste

7b6a03c

Catch a couple of additional leaked resources

f48c733

kristjanvalur force-pushed the kristjan/unittest-warning-cleanup branch from 11caef7 to f48c733 Compare July 25, 2022 10:18

dvora-h added the maintenance Maintenance (CI, Releases, etc) label Jul 26, 2022

dvora-h approved these changes Jul 26, 2022

View reviewed changes

dvora-h merged commit 6f20821 into redis:master Jul 26, 2022

kristjanvalur deleted the kristjan/unittest-warning-cleanup branch July 26, 2022 16:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix warnings and resource usage problems in asyncio unittests #2258

Fix warnings and resource usage problems in asyncio unittests #2258

kristjanvalur commented Jun 28, 2022 •

edited

Loading

codecov-commenter commented Jun 28, 2022 •

edited

Loading

dvora-h commented Jul 25, 2022

kristjanvalur commented Jul 25, 2022

kristjanvalur commented Jul 26, 2022

Fix warnings and resource usage problems in asyncio unittests #2258

Fix warnings and resource usage problems in asyncio unittests #2258

Conversation

kristjanvalur commented Jun 28, 2022 • edited Loading

Pull Request check-list

Description of change

codecov-commenter commented Jun 28, 2022 • edited Loading

Codecov Report

dvora-h commented Jul 25, 2022

kristjanvalur commented Jul 25, 2022

kristjanvalur commented Jul 26, 2022

kristjanvalur commented Jun 28, 2022 •

edited

Loading

codecov-commenter commented Jun 28, 2022 •

edited

Loading