Navigation Menu

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

Jump to bottom

Rewrite asyncio.to_thread tests to use IsolatedAsyncioTestCase #91260

Closed
asvetlov opened this issue Mar 23, 2022 · 10 comments
Closed

Rewrite asyncio.to_thread tests to use IsolatedAsyncioTestCase #91260

asvetlov opened this issue Mar 23, 2022 · 10 comments
Labels
3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir topic-asyncio

Comments

@asvetlov
Copy link
Contributor

BPO 47104
Nosy @asvetlov, @1st1, @miss-islington, @Fidget-Spinner
PRs
  • bpo-47104: Rewrite asyncio.to_thread tests to use IsolatedAsyncioTestCase #32086
  • [3.10] bpo-47104: Rewrite asyncio.to_thread tests to use IsolatedAsyncioTestCase (GH-32086) #32087
  • [3.9] bpo-47104: Rewrite asyncio.to_thread tests to use IsolatedAsyncioTestCase (GH-32086) #32088
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = None
created_at = <Date 2022-03-23.20:42:40.140>
labels = ['3.11', 'tests', '3.10', 'expert-asyncio']
title = 'Rewrite asyncio.to_thread tests to use IsolatedAsyncioTestCase'
updated_at = <Date 2022-04-06.17:56:00.892>
user = 'https://github.com/asvetlov'

    bugs.python.org fields:

    activity = <Date 2022-04-06.17:56:00.892>
actor = 'kj'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Tests', 'asyncio']
creation = <Date 2022-03-23.20:42:40.140>
creator = 'asvetlov'
dependencies = []
files = []
hgrepos = []
issue_num = 47104
keywords = ['patch']
message_count = 4.0
messages = ['415913', '415914', '416890', '416893']
nosy_count = 4.0
nosy_names = ['asvetlov', 'yselivanov', 'miss-islington', 'kj']
pr_nums = ['32086', '32087', '32088']
priority = 'normal'
resolution = None
stage = 'resolved'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue47104'
versions = ['Python 3.10', 'Python 3.11']
    @asvetlov asvetlov added 3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir topic-asyncio labels Mar 23, 2022
    @asvetlov
    Copy link
    Contributor Author

    New changeset ff619c7 by Andrew Svetlov in branch 'main':
    bpo-47104: Rewrite asyncio.to_thread tests to use IsolatedAsyncioTestCase (GH-32086)
    ff619c7

    @asvetlov asvetlov added 3.9 only security fixes labels Mar 23, 2022
    @miss-islington
    Copy link
    Contributor

    New changeset 9e1bfd8 by Miss Islington (bot) in branch '3.10':
    bpo-47104: Rewrite asyncio.to_thread tests to use IsolatedAsyncioTestCase (GH-32086)
    9e1bfd8

    @asvetlov asvetlov removed the 3.9 only security fixes label Mar 23, 2022
    @asvetlov asvetlov removed the 3.9 only security fixes label Mar 23, 2022
    @Fidget-Spinner
    Copy link
    Member

    Andrew, it seems that the post-commit CI has started failing after this commit. Specifically it says "test_asyncio failed (env changed)"

    From this page,
    https://github.com/python/cpython/commits/3.10?after=3856b4995ec0e632d47b733cdecb5183ac830568+34&branch=3.10

    the Ubuntu CI link for that commit:
    https://github.com/python/cpython/runs/5667655099?check_suite_focus=true

    Failing buildbots on 3.10:
    https://buildbot.python.org/all/#/release_status

    Sorry that I can't help more. I'm not an asyncio expert so I'm not sure why this is triggering.

    @Fidget-Spinner Fidget-Spinner reopened this Apr 6, 2022
    @Fidget-Spinner Fidget-Spinner reopened this Apr 6, 2022
    @Fidget-Spinner
    Copy link
    Member

    I forgot to specify, that this is *only* on 3.10, not main.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @kumaraditya303
    Copy link
    Contributor

    kumaraditya303 commented Apr 18, 2022 •
    edited

    The test is failing on GH Actions too on 3.10 branch, it should be reverted until a fix is found.

    See #91667

    @kumaraditya303 kumaraditya303 mentioned this issue Apr 18, 2022
    Closed
    @gpshead
    Copy link
    Member

    gpshead commented Apr 18, 2022 •
    edited

    Using python -m test.bisect_cmd test_asyncio --fail-env-changed in a 3.10 build I got:

    == Tests result: ENV CHANGED ==

1 test altered the execution environment:
    test_asyncio

Total duration: 2.7 sec
Tests result: ENV CHANGED
ran 2 tests/4
exit 3
Tests failed: continuing with this subtest

Tests (2):
* test.test_asyncio.test_threads.ToThreadTests.test_to_thread_concurrent
* test.test_asyncio.test_unix_events.SelectorEventLoopSignalTests.test_add_signal_handler

Bisection failed after 101 iterations and 0:04:02

    @gpshead
    Copy link
    Member

    gpshead commented Apr 18, 2022 

    0:00:00 load avg: 0.93 [1/2/1] test_asyncio.test_threads failed (env changed)
Warning -- threading._dangling was modified by test_asyncio.test_threads
  Before: {<weakref at 0x7f3b2e77fa10; to '_MainThread' at 0x7f3b2f107490>}
  After:  {<weakref at 0x7f3b2e77ffb0; to '_MainThread' at 0x7f3b2f107490>}

    @gpshead
    Copy link
    Member

    gpshead commented Apr 18, 2022

    I don't think this PR is the cause, just a code tweak that causes it to be exposed more often. Other tests lead to

    0:09:20 load avg: 1.49 [427/427/1] test_asyncio failed (env changed) (1 min 22 sec)
Warning -- threading_cleanup() failed to cleanup -1 threads (count: 0, dangling: 1)
Warning -- Dangling thread: <_MainThread(MainThread, started 140300013757632)>

    which is an environment change warning codepath from test.support.threading_helper which the test_asyncio.test_threads code no longer calls. from CI run https://github.com/python/cpython/runs/6069450575?check_suite_focus=true

    @gpshead gpshead mentioned this issue Apr 18, 2022
    Closed
    @gpshead
    Copy link
    Member

    gpshead commented Apr 18, 2022

    tracked in #91676 now as this issue's PR is merely a tickler of a latent problem, not the cause.

    @gpshead gpshead mentioned this issue Apr 19, 2022
    Merged
    pablogsal pushed a commit that referenced this issue Apr 19, 2022
    @gpshead
    [3.10] gh-91676 gh-91260 unittest.IsolatedAsyncioTestCase no longer l…
    61570ae 
    …eaks its executor (GH-91680)

For things like test_asyncio.test_thread this was causing frequent
"environment modified by test" errors as the executor threads had not
always stopped running after the test was over.
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Apr 19, 2022
    @gpshead @miss-islington
    [3.10] pythongh-91676 pythongh-91260 unittest.IsolatedAsyncioTestCase…
    d7f6bed 
    … no longer leaks its executor (pythonGH-91680)

For things like test_asyncio.test_thread this was causing frequent
"environment modified by test" errors as the executor threads had not
always stopped running after the test was over.
(cherry picked from commit 61570ae)

Co-authored-by: Gregory P. Smith <greg@krypto.org>
    @kumaraditya303
    Copy link
    Contributor

    Closing as it is now tracked in #91676

    miss-islington added a commit that referenced this issue Apr 19, 2022
    @miss-islington @gpshead
    [3.10] gh-91676 gh-91260 unittest.IsolatedAsyncioTestCase no longer l…
    cb3c85d 
    …eaks its executor (GH-91680)

For things like test_asyncio.test_thread this was causing frequent
"environment modified by test" errors as the executor threads had not
always stopped running after the test was over.
(cherry picked from commit 61570ae)

Co-authored-by: Gregory P. Smith <greg@krypto.org>
    hello-adam pushed a commit to hello-adam/cpython that referenced this issue Jun 2, 2022
    @miss-islington @gpshead @hello-adam
    [3.10] pythongh-91676 pythongh-91260 unittest.IsolatedAsyncioTestCase…
    ff493ea 
    … no longer leaks its executor (pythonGH-91680)

For things like test_asyncio.test_thread this was causing frequent
"environment modified by test" errors as the executor threads had not
always stopped running after the test was over.
(cherry picked from commit 61570ae)

Co-authored-by: Gregory P. Smith <greg@krypto.org>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir topic-asyncio
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    5 participants
    @gpshead @asvetlov @Fidget-Spinner @miss-islington @kumaraditya303