Set timeout only on test step by ShaharNaveh · Pull Request #6431 · RustPython/RustPython

ShaharNaveh · 2025-12-12T15:40:36Z

After seeing: https://github.com/RustPython/RustPython/actions/runs/20170296248/job/57903953379?pr=6423

Summary by CodeRabbit

Chores
- Updated CI/CD pipeline timeout configurations for improved test reliability across different platforms.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-12T15:40:48Z

Walkthrough

This pull request refactors CI/CD timeout configurations in the GitHub Actions workflow. It removes a global timeout from the rust_tests job and introduces granular, platform-specific per-step timeouts to the snippets_cpython job, with Windows tests allocated 45 minutes and Linux/macOS tests 35 minutes.

Changes

Cohort / File(s)	Summary
Workflow timeout configuration `\.github/workflows/ci\.yaml`	Removed global `timeout-minutes: 45` (Windows) and `timeout-minutes: 35` (other platforms) from `rust_tests` job; added per-step `timeout-minutes` to `snippets_cpython` job with platform-specific values (35 minutes for Linux and macOS, 45 minutes for Windows WINDOW_SKIPS tests)

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Single configuration file modified with straightforward timeout adjustments
No logic or functional changes; purely CI infrastructure tuning
Clear, self-evident modifications with consistent pattern across platforms

Possibly related PRs

CI: Increase windows timeout from 40 -> 45 #5993: Modifies the same workflow file to adjust Windows CI timeouts to 45 minutes, indicating related timeout tuning efforts.
Set timeout for CI #5947: Modifies the same workflow file and jobs (rust_tests and snippets_cpython) to adjust CI timeout configurations.

Suggested reviewers

youknowone

Poem

🐰 A rabbit hops through CI delays,
Adjusting timeouts, night and days,
Step by step, no more delays—
Windows gets forty-five, the hare conveys! ⏱️

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Set timeout only on test step' accurately reflects the main change: replacing global job-level timeouts with per-step timeouts in the CI workflow.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e227956 and e58c622.

📒 Files selected for processing (1)

.github/workflows/ci.yaml (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5947
File: .github/workflows/ci.yaml:116-116
Timestamp: 2025-07-11T09:35:43.571Z
Learning: GitHub Actions timeout-minutes field supports expressions that evaluate to integers, such as `${{ contains(matrix.os, 'windows') && 40 || 30 }}`, which will conditionally set different timeout values based on the runner OS.

📚 Learning: 2025-07-11T09:35:43.571Z

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5947
File: .github/workflows/ci.yaml:116-116
Timestamp: 2025-07-11T09:35:43.571Z
Learning: GitHub Actions timeout-minutes field supports expressions that evaluate to integers, such as `${{ contains(matrix.os, 'windows') && 40 || 30 }}`, which will conditionally set different timeout values based on the runner OS.

Applied to files:

.github/workflows/ci.yaml

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

GitHub Check: Check Rust code with clippy
GitHub Check: Run rust tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Run snippets and cpython tests on wasm-wasi
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Run rust tests (macos-latest)
GitHub Check: Ensure compilation on various targets
GitHub Check: Check the WASM package and demo

🔇 Additional comments (2)

.github/workflows/ci.yaml (2)

113-113: Inconsistency between AI summary and actual code changes.

The AI summary states "Removed a global timeout-minutes specification from the rust_tests job," but line 113 still contains timeout-minutes: ${{ contains(matrix.os, 'windows') && 45 || 35 }} at the job level. Additionally, the PR objectives state "Set timeout only on test step," but the rust_tests job retains its job-level timeout without corresponding per-step timeouts for individual test steps (lines 119–171).

Please clarify whether the global timeout at line 113 should be removed to align with the PR objective of setting timeouts only at the step level.

275-295: Step-level timeout configuration for snippets_cpython job looks good.

The per-step timeouts correctly apply 35 minutes to Linux and macOS test steps (lines 279, 284, 289) and 45 minutes to Windows (line 295), which appropriately reflects platform-specific test durations. The granular approach is cleaner than a single job-level timeout and gives each test stage its own time budget.

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

youknowone · 2025-12-13T00:42:49Z

still failed. probably macos runner was degraded

Set timeout only on test step

e58c622

youknowone merged commit 835918a into RustPython:main Dec 13, 2025
23 of 24 checks passed

coderabbitai bot mentioned this pull request Dec 26, 2025

increase timeout-minutes to 45 #6541

Merged

coderabbitai bot mentioned this pull request Jan 11, 2026

Upgrade threading to 3.13.11; sys.setprofile & impl more threading #6691

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set timeout only on test step#6431

Set timeout only on test step#6431
youknowone merged 1 commit intoRustPython:mainfrom
ShaharNaveh:ci-timeout-on-tests

ShaharNaveh commented Dec 12, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 12, 2025 •

edited

Loading

Uh oh!

youknowone commented Dec 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ShaharNaveh commented Dec 12, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

youknowone commented Dec 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ShaharNaveh commented Dec 12, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 12, 2025 •

edited

Loading