Fix update_lib tests by youknowone · Pull Request #6866 · RustPython/RustPython
Summary by CodeRabbit
- Tests
- Improved test coverage for Python module auto-detection, including validation of modules following specific naming patterns.
✏️ Tip: You can customize this high-level summary in your review settings.
📝 Walkthrough
Walkthrough
A test case has been replaced to validate auto-detection of Python modules matching the _py{module}.py pattern. The new test creates a temporary directory with datetime.py and _pydatetime.py, then verifies that get_lib_paths() returns both files.
Changes
| Cohort / File(s) | Summary |
|---|---|
Test auto-detection for Python module pattern scripts/update_lib/tests/test_deps.py |
Replaced test case asserting behavior for a known dependency with a new test validating auto-detection of _py{module}.py pattern modules; creates temporary Lib directory and verifies get_lib_paths() returns both datetime.py and _pydatetime.py |
Estimated code review effort
🎯 1 (Trivial) | ⏱️ ~5 minutes
Possibly related PRs
- update_lib hard dependency resolver #6817: The test change directly validates the dependency resolver's auto-detection behavior for
_py{module}.pyhard-dependencies introduced in that PR. - CI runs update_lib #6835: Related through shared test suite—adds CI step that runs
scripts/update_libtests that this change modifies.
Suggested reviewers
- moreal
- ShaharNaveh
Poem
🐰 A module hops through Lib with care,
DateTime and its twin pair,
Auto-detect, no need to guess,
Dependency patterns—nothing less!
_py friends now pass the test with flair ✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | ❓ Inconclusive | The title 'Fix update_lib tests' is vague and generic, not clearly describing the specific change being made. | Consider a more descriptive title such as 'Add auto-detection test for Python module patterns in update_lib' to clarify the main change. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/update_lib/tests/test_deps.py`:
- Around line 67-79: The new assertions and file creation in
test_auto_detect_py_module violate the repo's policy against adding or changing
test logic; revert this test change so the file only contains allowed edits
(e.g., expectedFailure decorators) or coordinate an exception with maintainers.
Specifically, undo the added creation of (lib_dir / "datetime.py") and (lib_dir
/ "_pydatetime.py") and the additional assertions around
get_lib_paths("datetime", ...) so test_auto_detect_py_module returns to its
prior state.
Comment on lines +67 to +79
| def test_auto_detect_py_module(self): | ||
| """Test auto-detection of _py{module}.py pattern.""" | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| tmpdir = pathlib.Path(tmpdir) | ||
| lib_dir = tmpdir / "Lib" | ||
| lib_dir.mkdir() | ||
| (lib_dir / "datetime.py").write_text("# datetime") | ||
| (lib_dir / "_pydatetime.py").write_text("# _pydatetime") | ||
|
|
||
| paths = get_lib_paths("datetime", str(tmpdir)) | ||
| self.assertEqual(len(paths), 2) | ||
| self.assertIn(tmpdir / "Lib" / "datetime.py", paths) | ||
| self.assertIn(tmpdir / "Lib" / "_pydatetime.py", paths) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test change violates test-file modification policy
Lines 67-79 introduce new test logic/assertions/data in a *test*.py file, which is disallowed by the repo’s test-file guidelines (only expectedFailure decorator changes are permitted). Please revert this test change or coordinate an exception with maintainers. As per coding guidelines, ...
🤖 Prompt for AI Agents
In `@scripts/update_lib/tests/test_deps.py` around lines 67 - 79, The new
assertions and file creation in test_auto_detect_py_module violate the repo's
policy against adding or changing test logic; revert this test change so the
file only contains allowed edits (e.g., expectedFailure decorators) or
coordinate an exception with maintainers. Specifically, undo the added creation
of (lib_dir / "datetime.py") and (lib_dir / "_pydatetime.py") and the additional
assertions around get_lib_paths("datetime", ...) so test_auto_detect_py_module
returns to its prior state.