Skip to content

Re-enable incline contact force test#2496

Open
camevor wants to merge 1 commit intonewton-physics:mainfrom
camevor:reenable-incline-contact-test
Open

Re-enable incline contact force test#2496
camevor wants to merge 1 commit intonewton-physics:mainfrom
camevor:reenable-incline-contact-test

Conversation

@camevor
Copy link
Copy Markdown
Member

@camevor camevor commented Apr 20, 2026

Description

After the mjwarp upgrade in #2245, this resolves #2239.

Checklist

  • New or existing tests cover these changes
  • The documentation is up to date with these changes
  • CHANGELOG.md has been updated (if user-facing change)

Summary by CodeRabbit

  • Tests
    • Re-enabled a previously skipped test to run on continuous integration, improving test coverage verification.

@camevor camevor self-assigned this Apr 20, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 96385d08-6153-48bd-824c-4a6c018496d1

📥 Commits

Reviewing files that changed from the base of the PR and between 89d07fb and ede198b.

📒 Files selected for processing (1)
  • newton/tests/test_mujoco_solver.py
💤 Files with no reviewable changes (1)
  • newton/tests/test_mujoco_solver.py

📝 Walkthrough

Walkthrough

Removed the skip decorator from test_contact_forces_on_incline test, re-enabling its execution on CI after being previously disabled due to intermittent failures documented in issue #2239.

Changes

Cohort / File(s) Summary
Test Skip Decorator Removal
newton/tests/test_mujoco_solver.py
Removed @unittest.skip("Flaky on CI, see GH-2239") decorator from test_contact_forces_on_incline, allowing the test to run on CI instead of being skipped.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR re-enables the test as stated but does not address the root cause (flaky behavior). Issue #2239 proposes multiple solutions including increased settling time, diagnostic logging, relaxed tolerance, or retry logic—none are implemented. Implement one of the proposed mitigations from #2239 (increase settling time, add retry logic, or diagnose the flakiness) before re-enabling the test to prevent CI failures.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: removing the skip decorator to re-enable the previously disabled test.
Out of Scope Changes check ✅ Passed The change is narrowly scoped to removing the skip decorator from one test, which aligns with the PR objective to re-enable the test.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: test_contact_forces_on_incline fails intermittently on CPU

2 participants