Skip to content

chore: test testcases with @unittest.skip decorator #5871

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

Jump to bottom
Merged
merged 1 commit into from
Jun 30, 2025
Merged

chore: test testcases with @unittest.skip decorator #5871

merged 1 commit into from
Jun 30, 2025

Conversation

moreal
Copy link
Contributor

@moreal moreal commented Jun 30, 2025 •
edited by coderabbitai bot
Loading

This pull request replaces the @unittest.skip decorator with @unittest.expectedFailure for test cases that are no longer failing due to panic. And, if the test case is no longer failing, their @unittest.skip decorators are just unmarked.

Claude Code prompt (Korean) @Lib/test 하위에 있는 테스트들중 `@unittest.skip` 으로 마킹되어 있고 메시지로 'panicked'를 포함하고 있는 것들을 골라 마킹을 하나씩 해제해보고 실제로 패닉이 발생하는지 확인하려고 합니다. 만약 패닉이 발생하지 않고 그저 실패한다면 `# TODO: RUSTPYTHON\n@unittest.expectedFailure`로 교체해야 합니다. `cargo run -q -- -m test --list-cases test.test_ast` 꼴로 전체 테스트 목록을 가져올 수 있고, `cargo run -q -- -m unittest ` 꼴로 특정 테스트를 실행해볼 수 있습니다. 우선 확인 해볼 테스트 케이스를 조사하여 PLAN.md에 체크리스트로 기록하여 주십시오.

Summary by CodeRabbit

  • Tests
    • Updated several previously skipped tests to be marked as expected failures, improving test reporting and visibility of known issues.
    • Enabled multiple tests that were previously skipped, allowing them to run during test execution.
    • Added comments to highlight areas with known issues for future reference.

@moreal
chore: test testcases with @unittest.skip decorator
8b90470 
Signed-off-by: Lee Dogeon <dev.moreal@gmail.com>
@coderabbitai coderabbitai
Copy link

coderabbitai bot commented Jun 30, 2025 •
edited
Loading

Walkthrough

Several Python standard library test files were updated to change how RustPython-specific issues are handled. Tests previously skipped due to RustPython panics or unimplemented features are now either marked as expected failures or have their skip decorators removed, allowing them to run. Comments indicating known RustPython issues were added or retained.

Changes

File(s) Change Summary
Lib/test/test_ast.py Replaced @unittest.skip with @unittest.expectedFailure for three tests; added # TODO: RUSTPYTHON comments.
Lib/test/test_cmd_line.py Changed skip to @unittest.expectedFailure for test_invalid_utf8_arg; added # TODO: RUSTPYTHON comment.
Lib/test/test_float.py Removed skip decorator and related comment from test_float_with_comma; test now runs normally.
Lib/test/test_io.py Changed two tests from @unittest.skip to @unittest.expectedFailure; added # TODO: RUSTPYTHON comments.
Lib/test/test_strftime.py Removed skip decorator from test_strftime; test now runs.
Lib/test/test_time.py Removed skip decorators from test_strftime_format_check and test_mktime_error; tests now run.

Poem

Oh, what a hop in the testing field,
Where skips once grew, now truths revealed!
Expected failures—let’s not hide,
For RustPython quirks, we’ll take in stride.
With every test that bravely runs,
The codebase grows, the rabbits have fun!
🐇✨

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b59a666 and 8b90470.

📒 Files selected for processing (6)
  • Lib/test/test_ast.py (2 hunks)
  • Lib/test/test_cmd_line.py (1 hunks)
  • Lib/test/test_float.py (0 hunks)
  • Lib/test/test_io.py (1 hunks)
  • Lib/test/test_strftime.py (0 hunks)
  • Lib/test/test_time.py (0 hunks)
💤 Files with no reviewable changes (3)
  • Lib/test/test_strftime.py
  • Lib/test/test_float.py
  • Lib/test/test_time.py
🧰 Additional context used
📓 Path-based instructions (2)
`Lib/**/*`: Files in the Lib/ directory (Python standard library copied from CPy...

Lib/**/*: Files in the Lib/ directory (Python standard library copied from CPython) should be edited very conservatively; modifications should be minimal and only to work around RustPython limitations.

📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)

List of files the instruction was applied to:

  • Lib/test/test_cmd_line.py
  • Lib/test/test_io.py
  • Lib/test/test_ast.py
`Lib/test/**/*`: Tests in Lib/test often use markers such as '# TODO: RUSTPYTHON...

Lib/test/**/*: Tests in Lib/test often use markers such as '# TODO: RUSTPYTHON', 'unittest.skip("TODO: RustPython ")', or 'unittest.expectedFailure' with a '# TODO: RUSTPYTHON ' comment when modifications are made.
NEVER comment out or delete any test code lines except for removing '@unittest.expectedFailure' decorators and upper TODO comments.
NEVER modify test assertions, test logic, or test data in Lib/test files.
The only acceptable modifications to test files are: (1) removing '@unittest.expectedFailure' decorators and the upper TODO comments when tests actually pass, (2) adding '@unittest.expectedFailure' decorators when tests cannot be fixed.
When a test cannot pass due to missing language features, keep it as expectedFailure and document the reason.

📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)

List of files the instruction was applied to:

  • Lib/test/test_cmd_line.py
  • Lib/test/test_io.py
  • Lib/test/test_ast.py
🧠 Learnings (4)
📓 Common learnings 
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When a test fails due to unsupported Python syntax or features, keep the test as '@unittest.expectedFailure' and document the reason, rather than modifying the test logic or data.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: Minimize modifications to CPython standard library files in the Lib/ directory; bug fixes should be made through Rust code modifications whenever possible.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: To find unimplemented methods and contribution opportunities, run './whats_left.py' in the RustPython repository.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When testing Python code for RustPython, always use the RustPython interpreter (via 'cargo run -- script.py') instead of the standard 'python' command.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: Follow Rust best practices for error handling and memory management in all RustPython Rust code.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When comparing behavior with CPython, use the 'python' command explicitly for CPython and 'cargo run --' for RustPython to avoid confusion.

Learnt from: moreal
PR: RustPython/RustPython#5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.

Learnt from: moreal
PR: RustPython/RustPython#5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: For custom Python code in RustPython, follow PEP 8 style and use ruff for linting.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: Always use 'cargo fmt' to format Rust code and follow the default rustfmt code style when contributing to RustPython.
Lib/test/test_cmd_line.py (7) 
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When a test fails due to unsupported Python syntax or features, keep the test as '@unittest.expectedFailure' and document the reason, rather than modifying the test logic or data.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: Minimize modifications to CPython standard library files in the Lib/ directory; bug fixes should be made through Rust code modifications whenever possible.

Learnt from: moreal
PR: RustPython/RustPython#5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When testing Python code for RustPython, always use the RustPython interpreter (via 'cargo run -- script.py') instead of the standard 'python' command.

Learnt from: moreal
PR: RustPython/RustPython#5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When comparing behavior with CPython, use the 'python' command explicitly for CPython and 'cargo run --' for RustPython to avoid confusion.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: To find unimplemented methods and contribution opportunities, run './whats_left.py' in the RustPython repository.
Lib/test/test_io.py (4)

undefined

<retrieved_learning>
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When a test fails due to unsupported Python syntax or features, keep the test as '@unittest.expectedFailure' and document the reason, rather than modifying the test logic or data.
</retrieved_learning>

<retrieved_learning>
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: Minimize modifications to CPython standard library files in the Lib/ directory; bug fixes should be made through Rust code modifications whenever possible.
</retrieved_learning>

<retrieved_learning>
Learnt from: moreal
PR: #5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
</retrieved_learning>

<retrieved_learning>
Learnt from: moreal
PR: #5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.
</retrieved_learning>

Lib/test/test_ast.py (6)

undefined

<retrieved_learning>
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When a test fails due to unsupported Python syntax or features, keep the test as '@unittest.expectedFailure' and document the reason, rather than modifying the test logic or data.
</retrieved_learning>

<retrieved_learning>
Learnt from: moreal
PR: #5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
</retrieved_learning>

<retrieved_learning>
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: Minimize modifications to CPython standard library files in the Lib/ directory; bug fixes should be made through Rust code modifications whenever possible.
</retrieved_learning>

<retrieved_learning>
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: To find unimplemented methods and contribution opportunities, run './whats_left.py' in the RustPython repository.
</retrieved_learning>

<retrieved_learning>
Learnt from: moreal
PR: #5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.
</retrieved_learning>

<retrieved_learning>
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When testing Python code for RustPython, always use the RustPython interpreter (via 'cargo run -- script.py') instead of the standard 'python' command.
</retrieved_learning>

🧬 Code Graph Analysis (1)
Lib/test/test_cmd_line.py (1)
Lib/unittest/case.py (1)
  • expectedFailure (183-185)
⏰ Context from checks skipped due to timeout of 90000ms (11)
  • GitHub Check: Run snippets and cpython tests on wasm-wasi
  • GitHub Check: Run tests under miri
  • GitHub Check: Run snippets and cpython tests (macos-latest)
  • GitHub Check: Run snippets and cpython tests (ubuntu-latest)
  • GitHub Check: Run snippets and cpython tests (windows-latest)
  • GitHub Check: Check the WASM package and demo
  • GitHub Check: Check Rust code with rustfmt and clippy
  • GitHub Check: Run rust tests (ubuntu-latest)
  • GitHub Check: Run rust tests (windows-latest)
  • GitHub Check: Run rust tests (macos-latest)
  • GitHub Check: Ensure compilation on various targets
🔇 Additional comments (6)
Lib/test/test_io.py (2)

5101-5102: LGTM! Correct transition from skip to expectedFailure.

The addition of the # TODO: RUSTPYTHON comment and @unittest.expectedFailure decorator follows the established pattern for handling RustPython-specific test issues. This change allows the test to run while properly documenting that it's expected to fail due to RustPython limitations.

5108-5109: LGTM! Consistent pattern applied.

The same appropriate pattern is applied here - replacing the skip with expectedFailure and adding the TODO comment. This maintains consistency with the other test method and follows the project's guidelines for handling RustPython-specific test failures.

Lib/test/test_cmd_line.py (1)

262-263: LGTM! Proper conversion from skip to expectedFailure.

This change correctly converts a test that previously caused panics to an expected failure, allowing it to run while acknowledging the known limitation. The addition follows the established pattern in the codebase and adheres to the coding guidelines for minimal test modifications.

Lib/test/test_ast.py (3)

343-344: LGTM! Proper conversion from skip to expected failure.

The addition of # TODO: RUSTPYTHON comment and @unittest.expectedFailure decorator follows the established pattern for RustPython test modifications. This correctly indicates that the test no longer panics but still fails due to unimplemented features.

357-358: LGTM! Consistent with RustPython test patterns.

The modification follows the same correct pattern as the previous test - adding the # TODO: RUSTPYTHON comment and @unittest.expectedFailure decorator without changing test logic.

366-367: LGTM! Maintains consistency across all changes.

This final modification completes the consistent pattern across all three test methods, properly marking them as expected failures while preserving the original test logic.

✨ 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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

youknowone
youknowone approved these changes Jun 30, 2025
View reviewed changes
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

👍 Thank you!

If you are looking for long-running prompt, I am building one.
https://gist.github.com/youknowone/7ec4d248e2064da23d87ccde20ceaa9b

@youknowone youknowone merged commit 5953a93 into RustPython:main Jun 30, 2025
12 checks passed
@moreal
Copy link
Contributor Author

moreal commented Jun 30, 2025

👍 Thank you!

If you are looking for long-running prompt, I am building one. gist.github.com/youknowone/7ec4d248e2064da23d87ccde20ceaa9b

Oh, thank you for sharing it! 🙇🏻‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youknowone youknowone youknowone approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@moreal @youknowone