[update_lib] deps grouping #6864
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR expands DEPENDENCIES with extensive per-module test mappings, changes is_up_to_date() to require actual CPython library paths to be present before reporting up-to-date, and adds test-tracking/grouping helpers to compute and format TODOs grouped by library. Changes
Sequence DiagramsequenceDiagram
participant Script as update_lib script
participant DEPS as DEPENDENCIES
participant FS as FileSystem (cpython repo)
participant Formatter as TodoFormatter
Script->>DEPS: Load module entries (incl. "test" lists)
Script->>FS: get_lib_paths(module)
alt lib paths found
FS-->>Script: lib paths list
Script->>FS: check CPython test file exists for each test
alt test exists
FS-->>Script: test path found
Script->>Formatter: mark test as tracked, assign lib_name & order
else test missing
FS-->>Script: not found
Script->>Formatter: mark test as untracked
end
Script->>Formatter: group tests by lib, sort by test_order
Formatter-->>Script: formatted grouped TODO lines with suffixes
else no lib paths
FS-->>Script: no paths
Script->>Formatter: skip dependency (no tests collected)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/update_lib/show_todo.py (1)
365-406: Update scoring docstring to match the new 0/1/2 scheme.The docstring still describes the old score meanings, while the code now uses 0 (ready), 1 (no lib), 2 (pending).
✏️ Docstring fix
- - If corresponding lib is NOT up-to-date: score = 1 (wait for lib) - - If no corresponding lib: score = -1 (independent) + - If corresponding lib is NOT up-to-date: score = 2 (wait for lib) + - If no corresponding lib: score = 1 (independent)scripts/update_lib/deps.py (1)
726-750: Handle modules with intentionally empty lib lists.Modules with
"lib": [](Rust-only implementations likeint,exception,dict,list) currently returnFalsefromis_up_to_date()sincefound_anyis never set toTrue. These modules have no CPython lib files to compare, so the empty result should be treated as up-to-date, not as pending. Without this fix, they appear as out-of-sync in dependent test views despite having no lib synchronization requirement.Consider adding explicit handling:
Suggested fix
- return found_any + if not found_any: + dep_info = DEPENDENCIES.get(name, {}) + if dep_info.get("lib") == []: + return True + return found_any
scripts/update_lib/show_todo.py
Outdated
| test_order = lib_test_order[lib_name].index(test_name) | ||
| else: | ||
| # Extract lib name from test name (test_foo -> foo) | ||
| lib_name = test_name[5:] if test_name.startswith("test_") else test_name |
There was a problem hiding this comment.
| lib_name = test_name[5:] if test_name.startswith("test_") else test_name | |
| lib_name = test_name.removeprefix("test_") |
2 participants
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.