Skip to content

set default timeout for pytest#5265

Open
link89 wants to merge 3 commits intodeepmodeling:masterfrom
link89:timeout-for-pytest
Open

set default timeout for pytest#5265
link89 wants to merge 3 commits intodeepmodeling:masterfrom
link89:timeout-for-pytest

Conversation

@link89
Copy link
Contributor

@link89 link89 commented Feb 25, 2026

Avoid wasting time on broken test that pending the workflow for a long time like this: https://github.com/deepmodeling/deepmd-kit/actions/runs/22224311896

Summary by CodeRabbit

  • Chores
    • Added test timeout configuration to prevent indefinitely hanging test runs, improving test suite reliability and execution stability.

Copilot AI review requested due to automatic review settings February 25, 2026 02:31
@dosubot dosubot bot added the enhancement label Feb 25, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a default timeout configuration for pytest to prevent tests from hanging indefinitely and blocking CI/CD workflows. The change addresses a specific issue where broken tests could cause workflows to pend for extended periods.

Changes:

  • Added pytest-timeout dependency to the test dependencies
  • Configured a default timeout of 1800 seconds (30 minutes) for all pytest tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 25, 2026

📝 Walkthrough

Walkthrough

Added pytest-timeout plugin as a test dependency and configured a global pytest timeout of 1800 seconds in the project configuration. These changes enable timeout enforcement for test execution across the test suite.

Changes

Cohort / File(s) Summary
Test Configuration
pyproject.toml
Added pytest-timeout dependency to test extras and configured global pytest timeout setting (1800 seconds).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title 'set default timeout for pytest' directly matches the main change in the pull request, which adds a global pytest timeout configuration.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
pyproject.toml (1)

458-458: Consider setting timeout_method = "thread" for harder CI-stall prevention.

If the intent is to reliably stop hangs in native/CUDA paths, explicitly using thread is usually more robust than default signal-based interruption.

♻️ Proposed update
 [tool.pytest.ini_options]
 markers = "run"
 timeout = 1800
+timeout_method = "thread"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pyproject.toml` at line 458, The CI timeout is set via timeout = 1800 but
lacks an explicit timeout_method; update pyproject.toml to add timeout_method =
"thread" alongside the existing timeout setting to ensure hangs in native/CUDA
code are interrupted reliably (modify the same section that contains the timeout
= 1800 entry to include timeout_method = "thread").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pyproject.toml`:
- Line 458: The CI timeout is set via timeout = 1800 but lacks an explicit
timeout_method; update pyproject.toml to add timeout_method = "thread" alongside
the existing timeout setting to ensure hangs in native/CUDA code are interrupted
reliably (modify the same section that contains the timeout = 1800 entry to
include timeout_method = "thread").

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 65eea4b and 73a861d.

📒 Files selected for processing (1)
  • pyproject.toml

@codecov
Copy link

codecov bot commented Feb 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.00%. Comparing base (65eea4b) to head (73a861d).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5265   +/-   ##
=======================================
  Coverage   82.00%   82.00%           
=======================================
  Files         750      750           
  Lines       75082    75082           
  Branches     3615     3616    +1     
=======================================
  Hits        61571    61571           
  Misses      12347    12347           
  Partials     1164     1164           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@link89 link89 changed the title set default ttimeout for pytest set default timeout for pytest Feb 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants