-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix update_lib tests #6866
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
Fix update_lib tests #6866
Conversation
📝 WalkthroughWalkthroughA test case has been replaced to validate auto-detection of Python modules matching the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
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.
| 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) |
There was a problem hiding this comment.
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.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.