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

Modify test_codecs to use the new codecs.unregister() function #86085

Closed
shihai1991 opened this issue Oct 3, 2020 · 6 comments
Closed

Modify test_codecs to use the new codecs.unregister() function #86085

shihai1991 opened this issue Oct 3, 2020 · 6 comments
Labels
3.10 only security fixes tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@shihai1991
Copy link
Member

shihai1991 commented Oct 3, 2020

BPO 41919
Nosy @vstinner, @pablogsal, @shihai1991
PRs
  • bpo-41919: Move the codecs.register operation to the setup of testcases. #22513
  • bpo-41919: Revert "bpo-41919: Move the codecs.register operation to the setup of testcases." #22961
  • bpo-41919: Avoid resource leak in test_io module #22973
  • 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 = <Date 2020-10-25.18:38:56.606>
    created_at = <Date 2020-10-03.09:37:07.680>
    labels = ['type-feature', 'tests', '3.10']
    title = 'Modify test_codecs to use the new codecs.unregister() function'
    updated_at = <Date 2020-10-25.18:38:56.605>
    user = 'https://github.com/shihai1991'

    bugs.python.org fields:

    activity = <Date 2020-10-25.18:38:56.605>
    actor = 'pablogsal'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-10-25.18:38:56.606>
    closer = 'pablogsal'
    components = ['Tests']
    creation = <Date 2020-10-03.09:37:07.680>
    creator = 'shihai1991'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 41919
    keywords = ['patch']
    message_count = 6.0
    messages = ['377863', '378708', '379565', '379567', '379584', '379598']
    nosy_count = 3.0
    nosy_names = ['vstinner', 'pablogsal', 'shihai1991']
    pr_nums = ['22513', '22961', '22973']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue41919'
    versions = ['Python 3.10']

    Linked PRs

    @shihai1991
    Copy link
    Member Author

    After PR22360 merged, we can move the codecs' register operation to testcases.

    @shihai1991 shihai1991 added 3.10 only security fixes tests Tests in the Lib/test dir type-feature A feature request or enhancement labels Oct 3, 2020
    @shihai1991 shihai1991 changed the title Move the codecs' register operation to testcases Move the codecs.register operation to testcases Oct 3, 2020
    @shihai1991 shihai1991 changed the title Move the codecs' register operation to testcases Move the codecs.register operation to testcases Oct 3, 2020
    @vstinner vstinner changed the title Move the codecs.register operation to testcases Modify test_codecs to use the new codecs.unregister() function Oct 3, 2020
    @vstinner vstinner changed the title Move the codecs.register operation to testcases Modify test_codecs to use the new codecs.unregister() function Oct 3, 2020
    @vstinner
    Copy link
    Member

    New changeset c9f696c by Hai Shi in branch 'master':
    bpo-41919, test_codecs: Move codecs.register calls to setUp() (GH-22513)
    c9f696c

    @pablogsal
    Copy link
    Member

    Commit c9f696c introduced a reference leak in the test suite. See https://bugs.python.org/issue42145:

    c9f696c is the first bad commit
    commit c9f696c
    Author: Hai Shi <shihai1992@gmail.com>
    Date: Fri Oct 16 16:34:15 2020 +0800

    bpo-41919, test_codecs: Move codecs.register calls to setUp() (GH-22513)
    

    Reverting the commit eliminates the problem:

    rences, sum=12
    test_io leaked [1, 1, 1, 1] memory blocks, sum=4
    test_io failed

    == Tests result: FAILURE ==

    1 test failed:
    test_io

    Total duration: 397 ms
    Tests result: FAILURE
    g
    ~/github/python/master master|bisect*
    ❯ git revert c9f696c
    Auto-merging Lib/test/test_codecs.py
    [master f3de7c00b4] Revert "bpo-41919, test_codecs: Move codecs.register calls to setUp() (GH-22513)"
    7 files changed, 112 insertions(+), 16 deletions(-)

    ~/github/python/master master|bisect* ⇡
    ❯ make -j -s
    CC='gcc -pthread' LDSHARED='gcc -pthread -shared ' OPT='-g -Og -Wall' _TCLTK_INCLUDES='' _TCLTK_LIBS='' ./python -E ./setup.py -q build

    The following modules found by detect_modules() in setup.py, have been
    built by the Makefile instead, as configured by the Setup files:
    _abc atexit pwd
    time

    ~/github/python/master master|bisect* ⇡
    ❯ ./python -m test test_io -m test.test_io.CTextIOWrapperTest.test_read_one_by_one -R :
    0:00:00 load avg: 2.57 Run tests sequentially
    0:00:00 load avg: 2.57 [1/1] test_io
    beginning 9 repetitions
    123456789
    .........

    == Tests result: SUCCESS ==

    1 test OK.

    Total duration: 455 ms
    Tests result: SUCCESS

    * Move the codecs' (un)register operation to testcases.
    * Remove _codecs._forget_codec() and _PyCodec_Forget()
    

    Lib/test/test_charmapcodec.py | 7 +++++--
    Lib/test/test_codecs.py | 25 +++----------------------
    Lib/test/test_io.py | 7 +++----
    Lib/test/test_unicode.py | 5 ++++-
    Modules/_codecsmodule.c | 20 --------------------
    Modules/clinic/_codecsmodule.c.h | 39 +--------------------------------------
    Python/codecs.c | 25 -------------------------
    7 files changed, 16 insertions(+), 112 deletions(-)

    @pablogsal
    Copy link
    Member

    AS this is masking other issues in the build bots, we need to revert the commit unless is fixed in 24 hours per the buildbot workflow

    @pablogsal pablogsal reopened this Oct 25, 2020
    @pablogsal pablogsal reopened this Oct 25, 2020
    @shihai1991
    Copy link
    Member Author

    AS this is masking other issues in the build bots, we need to revert the commit unless is fixed in 24 hours per the buildbot workflow

    Thanks, Pablo. I checked that only test_io.py have resource leak.
    I create PR-22973, pls take a look if you have free time, thanks.

    @pablogsal
    Copy link
    Member

    New changeset 14cdc21 by Hai Shi in branch 'master':
    bpo-41919: Avoid resource leak in test_io (GH-22973)
    14cdc21

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    vstinner added a commit to vstinner/cpython that referenced this issue Jul 3, 2023
    The code was already removed by:
    commit c9f696c.
    vstinner added a commit that referenced this issue Jul 3, 2023
    The code was already removed by:
    commit c9f696c.
    carljm added a commit to carljm/cpython that referenced this issue Jul 4, 2023
    * main:
      pythongh-106368: Increase Argument Clinic test coverage (python#106389)
      pythongh-106320: Fix _PyImport_GetModuleAttr() declaration (python#106386)
      pythongh-106368: Harden Argument Clinic parser tests (python#106384)
      pythongh-106320: Remove private _PyImport C API functions (python#106383)
      pythongh-86085: Remove _PyCodec_Forget() declaration (python#106377)
      pythongh-106320: Remove more private _PyUnicode C API functions (python#106382)
      pythongh-104050: Annotate more Argument Clinic DSLParser state methods (python#106376)
      pythongh-106368: Clean up Argument Clinic tests (python#106373)
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes tests Tests in the Lib/test dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants