Skip to content

Fix update_lib marker preservation for inline classes #9131

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.

Sign up for GitHub

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
youknowone merged 2 commits into from
Oct 15, 2026
Merged

Fix update_lib marker preservation for inline classes #9131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
609172c
Preserve inherited test markers in inline class migrations
youknowdot Oct 15, 2026
ad6ed9b
Run migration tool tests through existing CI conditions
youknowdot Oct 15, 2026
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
33 changes: 28 additions & 5 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ jobs:
outputs:
# Flag that is raised when any rust code is changed.
rust_code: ${{ steps.check_rust_code.outputs.changed }}
update_lib: ${{ steps.check_update_lib.outputs.changed }}
# WASM demo / notebook / example_projects, independent of rust_code.
wasm_code: ${{ steps.check_wasm_code.outputs.changed }}
# Cross-target cargo check is skipped unless the diff can change
Expand Down Expand Up @@ -87,6 +88,17 @@ jobs:
env:
MERGE_BASE: ${{ steps.merge_base.outputs.sha }}

- name: Check if update_lib changed
id: check_update_lib
run: |
if git diff --quiet "${MERGE_BASE}...HEAD" -- ':scripts/update_lib/**' ':.python-version'; then
echo "changed=false" >> "$GITHUB_OUTPUT"
else
echo "changed=true" >> "$GITHUB_OUTPUT"
fi
env:
MERGE_BASE: ${{ steps.merge_base.outputs.sha }}

- name: Check if WASM package or demo sources changed
id: check_wasm_code
run: |
Expand Down Expand Up @@ -209,6 +221,7 @@ jobs:
env:
RUST_BACKTRACE: full
RUN_RUST_TESTS: ${{ needs.determine_changes.outputs.rust_code == 'true' || github.ref == 'refs/heads/main' }}
RUN_UPDATE_LIB_TESTS: ${{ needs.determine_changes.outputs.update_lib == 'true' }}
# Named after the matrix entry rather than left to be named for it: a
# generated name lists every value in the entry, so adding or removing one
# renames the check and drops it from the required list.
Expand All @@ -222,10 +235,10 @@ jobs:
steps:
- name: Skip rust tests when sources did not change
if: env.RUN_RUST_TESTS != 'true'
run: echo "No rust source changes; required check is green without rebuilding."
run: echo "No rust source changes; skipping Rust build and tests."

- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
if: env.RUN_RUST_TESTS == 'true'
if: env.RUN_RUST_TESTS == 'true' || (env.RUN_UPDATE_LIB_TESTS == 'true' && runner.os == 'Linux')
with:
persist-credentials: false

Expand Down Expand Up @@ -345,9 +358,19 @@ jobs:
if: env.RUN_RUST_TESTS == 'true' && runner.os == 'Linux'
run: cargo build --locked

- uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
if: env.RUN_RUST_TESTS != 'true' && env.RUN_UPDATE_LIB_TESTS == 'true' && runner.os == 'Linux'
with:
python-version-file: .python-version

- name: run update_lib tests
if: env.RUN_RUST_TESTS == 'true' && runner.os == 'Linux'
run: target/debug/rustpython -m unittest discover -s scripts/update_lib/tests -v
if: (env.RUN_RUST_TESTS == 'true' || env.RUN_UPDATE_LIB_TESTS == 'true') && runner.os == 'Linux'
run: |
if [ "$RUN_RUST_TESTS" = "true" ]; then
target/debug/rustpython -m unittest discover -s scripts/update_lib/tests -v
else
python -m unittest update_lib.tests.test_patch_spec update_lib.tests.test_migrate -v
fi
env:
PYTHONPATH: scripts

Expand Down Expand Up @@ -1090,7 +1113,7 @@ jobs:
steps:
- name: Skip wasm-wasi when rust sources did not change
if: env.RUN_WASM_WASI != 'true'
run: echo "No rust source changes; required check is green without rebuilding."
run: echo "No rust source changes; skipping Rust build and tests."

- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
if: env.RUN_WASM_WASI == 'true'
Expand Down
29 changes: 29 additions & 0 deletions scripts/update_lib/patch_spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,8 +414,36 @@ def _find_import_insert_line(tree: ast.Module) -> int:
return 0


def _expand_inline_class_suites(contents: str, patches: Patches) -> str:
"""Make room for inherited overrides without changing existing class bodies."""
tree = ast.parse(contents)
lines = contents.splitlines()
replacements = []
for node in _iter_module_classes(tree):
methods = {method.name for method in _iter_class_methods(node)}
inherited = set(patches.get(node.name, {})) - methods - {"__self__"}
if not inherited:
continue
first = node.body[0]
line = lines[first.lineno - 1].encode("utf-8")
header = line[: first.col_offset].decode("utf-8")
if not header.strip():
continue
body = line[first.col_offset :].decode("utf-8")
replacements.append(
(
first.lineno - 1,
header.rstrip() + "\n" + " " * (node.col_offset + 4) + body,
)
)
for index, replacement in sorted(replacements, reverse=True):
lines[index] = replacement
return "\n".join(lines) + "\n"


def apply_patches(contents: str, patches: Patches) -> str:
"""Apply patches to file contents and return modified contents."""
contents = _expand_inline_class_suites(contents, patches)
tree = ast.parse(contents)
lines = contents.splitlines()

Expand All @@ -436,6 +464,7 @@ def apply_patches(contents: str, patches: Patches) -> str:
lines.insert(lineno, patch)

joined = "\n".join(lines)
ast.parse(joined)
return f"{joined}\n"


Expand Down
33 changes: 33 additions & 0 deletions scripts/update_lib/tests/test_patch_spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,39 @@ def test_one(self):
self.assertIn("@some_decorator", result)
self.assertIn("@unittest.expectedFailure", result)

def test_inherited_override_expands_inline_class(self):
for source in (
"class Child(Base): pass # keep comment\n",
"class Child(Base): value = 1; other = 2\n",
"class Child(\n Base\n): pass\n",
"if True:\n class Child(Base): pass\n",
"class Café(Base): pass\n",
):
with self.subTest(source=source):
name = "Café" if "Café" in source else "Child"
patches = {
name: {
"test_parent": [PatchSpec(UtMethod.ExpectedFailure, None, "")]
}
}
before = ast.parse(source)
result = apply_patches(source, patches)
after = ast.parse(result)
old_class = next(
node for node in ast.walk(before) if isinstance(node, ast.ClassDef)
)
new_class = next(
node for node in ast.walk(after) if isinstance(node, ast.ClassDef)
)
self.assertEqual(
[ast.dump(node) for node in old_class.body],
[ast.dump(node) for node in new_class.body[:-1]],
)
self.assertEqual(new_class.body[-1].name, "test_parent")
self.assertIn("return super().test_parent()", result)
if "# keep comment" in source:
self.assertIn("# keep comment", result)

def test_apply_inherited_method(self):
"""Test applying patch to inherited method (creates override)."""
code = """import unittest
Expand Down
Loading