Skip to content

Stop skipping six tests whose reason no longer reproduces - #8583

Merged
youknowone merged 1 commit into
RustPython:mainfrom
luantaraschi:unskip-passing-tests
Aug 22, 2026
Merged

Stop skipping six tests whose reason no longer reproduces#8583
youknowone merged 1 commit into
RustPython:mainfrom
luantaraschi:unskip-passing-tests

Conversation

@luantaraschi

@luantaraschi luantaraschi commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Six tests are skipped for something that no longer happens.

file tests reason on the marker
Lib/test/test_array.py test_setitem_use_after_clear_with_int_data, test_setitem_use_after_shrink_with_int_data, test_setitem_use_after_clear_with_float_data TODO: RUSTPYTHON; Hangs
Lib/test/test_marshal.py testRecursion TODO: RUSTPYTHON
Lib/test/test_thread.py test_start_duplicate_handle, test_start_with_none_handle TODO: RUSTPYTHON; panic, on Linux only

What was measured

The test_array three cover array.__setitem__ when the index conversion mutates the array, from python/cpython#142555. Before unskipping them I ran their bodies straight against CPython 3.14, one process each with a timeout, plus four shapes the tests do not cover:

case CPython RustPython
clear() in __index__, then victim[1] = idx IndexError, len 0 IndexError, len 0
two pop() in __index__ IndexError, len 1 IndexError, len 1
clear() in __float__ on a 'd' array IndexError, len 0 IndexError, len 0
append() in __index__ [1, 0, 3, 9] [1, 0, 3, 9]
clear() in __index__ on a read IndexError IndexError
clear() in __index__ on a slice assignment [7] [7]
clear() in __index__ on del IndexError IndexError

Nothing hangs, and the exception is the one the tests assert on. The exception text differs (array assignment index out of range against assignment index out of range), which the tests do not look at, and which belongs to #7993 rather than here.

testRecursion marshals a self-referential dict and list through helper3. It passes.

The two in test_thread exercise thread.start_joinable_thread with a reused _ThreadHandle and with handle=None. They already ran on macOS and Windows; only the Linux skip is dropped, and they passed there five times in a row.

I did not track down which change fixed each one, so the claim here is only that the stated reason does not reproduce on current main.

Runs

Each module ran three times end to end after the markers came off, and the two thread tests ran five times:

[1] test_array -> Total tests: run=892 skipped=45 Result: SUCCESS
[1] test_marshal -> Total tests: run=75 skipped=15 Result: SUCCESS
[1] test_thread -> Total tests: run=35 skipped=3 Result: SUCCESS

scripts/check_redundant_patches.py is clean on all three files.

Left alone

test_logging has two markers reading Flaky and flaky EOFError, and test_thread.test__count reads Flakey on CI. All three passed here, and that is not evidence about a flake, so they stay as they are.

@luantaraschi
Stop skipping six tests whose reason no longer reproduces
012b0c9 
Three skips in test_array say "Hangs", one in test_marshal carries no
reason at all, and two in test_thread say "panic" on Linux. None of the
six does any of that now. Each ran three to five times in a row and
passed, and the array trio was also checked case by case against CPython
3.14, including the mutation-during-index-conversion shapes the tests
were written for.

The neighbouring markers that say "Flaky", "Flakey on CI" and "flaky
EOFError" are left alone. One green run says nothing about those.

Assisted-by: Claude Code:claude-opus-5
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (3)
  • Lib/test/test_array.py is excluded by !Lib/**
  • Lib/test/test_marshal.py is excluded by !Lib/**
  • Lib/test/test_thread.py is excluded by !Lib/**

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4d837ca9-0b3a-478a-b84a-3e6cbab91b3c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] test: cpython/Lib/test/test_marshal.py (TODO: 4)

dependencies:

dependent tests: (25 tests)

  • marshal: test_bool test_exceptions test_importlib test_inspect test_marshal test_zipimport
    • importlib._bootstrap_external: test_importlib test_unittest
      • modulefinder: test_importlib test_modulefinder
      • py_compile: test_argparse test_cmd_line_script test_compileall test_importlib test_multiprocessing_main_handling test_py_compile test_pydoc test_runpy
      • pydoc: test_enum
    • pkgutil: test_pkgutil test_pyrepl
    • profile: test_profile
    • pstats: test_pstats
    • zipimport: test_importlib test_zipimport_support

[x] test: cpython/Lib/test/test_array.py

dependencies:

dependent tests: (102 tests)

  • array: test_android test_array test_base64 test_binascii test_buffer test_bytes test_bz2 test_codecs test_collections test_csv test_ctypes test_file test_fileio test_float test_genericalias test_gzip test_hashlib test_httplib test_int test_io test_ioctl test_long test_lzma test_marshal test_memoryio test_memoryview test_patma test_re test_reprlib test_socket test_sqlite3 test_ssl test_struct test_subprocess test_urllib2 test_zipfile test_zstd
    • socket: test_asyncio test_epoll test_exception_hierarchy test_external_inspection test_ftplib test_httpservers test_imaplib test_kqueue test_largefile test_logging test_mailbox test_mmap test_os test_pathlib test_poplib test_pty test_selectors test_signal test_smtplib test_smtpnet test_socketserver test_stat test_support test_sys test_timeout test_urllib test_urllib2net test_urllib_response test_urllibnet test_xmlrpc
      • asyncio: test_asyncio test_inspect test_pdb test_unittest
      • email.utils: test_email
      • http.client: test_docxmlrpc test_ucn test_unicodedata test_wsgiref
      • http.server: test_robotparser test_urllib2_localnet
      • logging.handlers: test_concurrent_futures test_pkgutil
      • platform: test__locale test__osx_support test_baseexception test_builtin test_cmath test_ctypes test_fcntl test_math test_mimetypes test_platform test_posix test_regrtest test_shutil test_strptime test_sysconfig test_time test_winreg
      • ssl: test_venv
      • urllib.request: test_http_cookiejar test_pydoc test_sax test_site

[x] test: cpython/Lib/test/test_thread.py (TODO: 1)
[x] test: cpython/Lib/test/test_thread_local_bytecode.py
[x] test: cpython/Lib/test/test_threadsignals.py

dependencies:

dependent tests: (14 tests)
- [ ] concurrent.futures: test_asyncio test_compileall test_concurrent_futures test_context test_genericalias test_inspect test_struct test_wmi
- [ ] asyncio: test_asyncio test_external_inspection test_logging test_os test_pdb test_unittest

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@youknowone
youknowone merged commit 4e2ac38 into RustPython:main Aug 22, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@luantaraschi @youknowone