Skip to content

[mypyc] Fix remaining failing test on free-threaded builds #19646

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

Merged
merged 1 commit into from
Aug 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mypyc/test-data/irbuild-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ L0:
o.x = r1; r2 = is_error
return o

[case testSubclass_toplevel]
[case testSubclass_withgil_toplevel]
from typing import TypeVar, Generic
from mypy_extensions import trait
T = TypeVar('T')
Expand Down
5 changes: 4 additions & 1 deletion mypyc/test/test_irbuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from mypy.errors import CompileError
from mypy.test.config import test_temp_dir
from mypy.test.data import DataDrivenTestCase
from mypyc.common import TOP_LEVEL_NAME
from mypyc.common import IS_FREE_THREADED, TOP_LEVEL_NAME
from mypyc.ir.pprint import format_func
from mypyc.test.testutil import (
ICODE_GEN_BUILTINS,
Expand Down Expand Up @@ -71,6 +71,9 @@ def run_case(self, testcase: DataDrivenTestCase) -> None:
if options is None:
# Skipped test case
return
if "_withgil" in testcase.name and IS_FREE_THREADED:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, as an option, you can add an opposite marker _withnogil, and have two copies of the problematic test with both markers and the different outputs for each marker (I remember we had something similar for daemon, where the order of error messages depends on whether we use cache or not).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a good idea. In this case the free threading variant doesn't seem super useful, and a better way to test for the free threading behavior would be to have multi-threaded perf scaling benchmarks. But in the future it seems likely that we'll want separate no-GIL irbuild test cases, and we can implement your idea then.

# Test case should only run on a non-free-threaded build.
return
with use_custom_builtins(os.path.join(self.data_prefix, ICODE_GEN_BUILTINS), testcase):
expected_output = remove_comment_lines(testcase.output)
expected_output = replace_word_size(expected_output)
Expand Down
Loading