Disable flaky barrier tests #520

rgsl888prabhu · 2025-10-21T15:08:02Z

Description

This PR disables few barrier tests which are flaky and causes segfault, there is an issue for tracking this, and once the issue is resolved, these tests should be unblocked.

Issue

#519

Checklist

I am familiar with the Contributing Guidelines.
Testing
- New or existing tests cover these changes
- Added tests
- Created an issue to follow-up
- NA
Documentation
- The documentation is up to date with these changes
- Added new documentation
- NA

Summary by CodeRabbit

Tests
- The barrier-solver test is now explicitly skipped with an annotated reason referencing a related issue.
- The test remains parameterized exactly as before; no changes to parameters or test logic.

coderabbitai · 2025-10-21T15:08:35Z

Walkthrough

Added an active skip decorator to test_barrier_solver_options in python/cuopt_server/cuopt_server/tests/test_lp.py with a reason pointing to issue #519; the test remains parameterized and its logic was not otherwise changed.

Changes

Cohort / File(s)	Summary
Test skip decorator `python/cuopt_server/cuopt_server/tests/test_lp.py`	Replaced a previously commented skip-marker approach by applying an active `@pytest.mark.skip` (or equivalent) to `test_barrier_solver_options`, including a reason referencing issue #519. Test parameters and internal logic unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 I hopped into tests with a tiny sigh,

A skip I placed where odd tuples lie,
I leave a note to track the mend,
Till issue fixed, this pause I lend,
🥕 Soft thumps, I guard the test supply.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "Disable flaky barrier tests" directly aligns with the main change in the changeset, which replaces a commented barrier solver options skip marker with an active skip decorator on the test_barrier_solver_options test. The title is clear, concise, and specific—it indicates both the action (disable) and the target (flaky barrier tests)—which matches the PR's objective of disabling tests that are flaky and cause segmentation faults. A teammate scanning the repository history would immediately understand that this change disables problematic barrier tests.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

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

📜 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 c50950d and 0c8550b.

📒 Files selected for processing (1)

python/cuopt_server/cuopt_server/tests/test_lp.py (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

python/cuopt_server/cuopt_server/tests/test_lp.py

⏰ 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). (8)

GitHub Check: wheel-build-libcuopt / 13.0.1, 3.13, arm64, rockylinux8
GitHub Check: wheel-build-libcuopt / 12.9.1, 3.13, amd64, rockylinux8
GitHub Check: wheel-build-libcuopt / 12.9.1, 3.13, arm64, rockylinux8
GitHub Check: wheel-build-libcuopt / 13.0.1, 3.13, amd64, rockylinux8
GitHub Check: conda-cpp-build / 12.9.1, 3.10, arm64, rockylinux8
GitHub Check: conda-cpp-build / 12.9.1, 3.10, amd64, rockylinux8
GitHub Check: conda-cpp-build / 13.0.1, 3.10, amd64, rockylinux8
GitHub Check: conda-cpp-build / 13.0.1, 3.10, arm64, rockylinux8

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

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

python/cuopt_server/cuopt_server/tests/test_lp.py (1)
158-165: LGTM! Appropriate approach to disable flaky tests.

Disabling these flaky tests while tracking the issue is the right approach. The issue reference is properly included.

Consider consolidating the duplicate comments to reduce repetition:
 # Enable barrier solver options when this issue is resolved
 # https://github.com/NVIDIA/cuopt/issues/519
 # Test folding off, no dualization, cuDSS default ordering, ADAT system
 # (0, 0, 0, 0, True, False, 0),
-# Enable barrier solver options when this issue is resolved
-# https://github.com/NVIDIA/cuopt/issues/519
 # Test folding on, force dualization, AMD ordering, augmented system
 # (1, 1, 1, 1, True, True, 1),

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 052aea8 and 3b890c2.

📒 Files selected for processing (1)

python/cuopt_server/cuopt_server/tests/test_lp.py (1 hunks)

⏰ 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: wheel-build-cuopt-mps-parser / 13.0.1, 3.13, arm64, rockylinux8
GitHub Check: wheel-build-cuopt-mps-parser / 13.0.1, 3.11, amd64, rockylinux8
GitHub Check: wheel-build-cuopt-mps-parser / 13.0.1, 3.13, amd64, rockylinux8
GitHub Check: wheel-build-cuopt-mps-parser / 13.0.1, 3.10, arm64, rockylinux8
GitHub Check: wheel-build-cuopt-mps-parser / 13.0.1, 3.12, arm64, rockylinux8
GitHub Check: wheel-build-cuopt-mps-parser / 13.0.1, 3.12, amd64, rockylinux8
GitHub Check: wheel-build-cuopt-mps-parser / 13.0.1, 3.11, arm64, rockylinux8
GitHub Check: wheel-build-cuopt-mps-parser / 13.0.1, 3.10, amd64, rockylinux8
GitHub Check: wheel-build-cuopt-sh-client / 13.0.1, 3.10, amd64, rockylinux8
GitHub Check: checks / check-style

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

python/cuopt_server/cuopt_server/tests/test_lp.py (1)

151-151: Consider removing the commented skip marker.

The commented-out skip marker suggests there was prior consideration to disable the entire test. Since you've opted for a more targeted approach (disabling only the flaky parameter combinations), consider removing this commented line to avoid confusion.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3b890c2 and 4fb9cbf.

📒 Files selected for processing (1)

python/cuopt_server/cuopt_server/tests/test_lp.py (1 hunks)

🔇 Additional comments (1)

python/cuopt_server/cuopt_server/tests/test_lp.py (1)

158-163: Incorrect line number references and incomplete context.

The original review comment contains inaccurate line number citations. The enabled test parameters are not at lines 157, 165, 167, 169, 171. More importantly, this change represents a re-enabling of a previously completely skipped test (@pytest.mark.skip is now commented out on line 150), with selective disabling of only the two problematic parameter combinations related to issue #519. This is a more significant positive change than the review acknowledges.

The request to verify remaining parameters won't exhibit segfaults is a runtime behavior concern that cannot be resolved through static code analysis.

Likely an incorrect or invalid review comment.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

python/cuopt_server/cuopt_server/tests/test_lp.py (1)
157-162: Good approach to temporarily disable flaky test cases.

The surgical fix correctly disables only the problematic parameter combinations while maintaining test coverage with the remaining 5 configurations. The inline comments appropriately document the temporary nature and reference issue #519 for tracking.

Optional enhancement for discoverability:

Consider adding a TODO or FIXME marker to make these disabled tests more discoverable in code searches:
-        # Enable barrier solver options when this issue is resolved
+        # TODO: Enable barrier solver options when this issue is resolved
         # https://github.com/NVIDIA/cuopt/issues/519

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fb9cbf and c50950d.

📒 Files selected for processing (1)

python/cuopt_server/cuopt_server/tests/test_lp.py (1 hunks)

rgsl888prabhu · 2025-10-21T15:39:22Z

@CodeRabbit resolve all comments and approve

coderabbitai · 2025-10-21T15:40:22Z

🧩 Analysis chain

@rgsl888prabhu: I'll resolve all my comments and approve the PR for you.

🏁 Script executed:

#!/bin/bash

# List all review comments on PR #520 to find comments made by coderabbitai
echo "Fetching review comments on PR #520..."
gh pr view 520 --json reviews --jq '.reviews[] | select(.author.login == "coderabbitai") | {id: .id, state: .state, body: .body}'

echo ""
echo "---"
echo ""

# List all issue comments on PR #520 to find comments made by coderabbitai
echo "Fetching issue comments on PR #520..."
gh pr view 520 --json comments --jq '.comments[] | select(.author.login == "coderabbitai") | {id: .id, body: .body}'

Length of output: 18795

🏁 Script executed:

#!/bin/bash

# Check for review threads/conversations on PR #520
echo "Checking for review threads on PR #520..."
gh api graphql -f query='
  query {
    repository(owner: "NVIDIA", name: "cuopt") {
      pullRequest(number: 520) {
        reviewThreads(first: 100) {
          nodes {
            id
            isResolved
            comments(first: 10) {
              nodes {
                author {
                  login
                }
                body
              }
            }
          }
        }
      }
    }
  }
' --jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | select(.comments.nodes[0].author.login == "coderabbitai")'

Length of output: 41

@rgsl888prabhu: I've checked for review comments on PR #520. There are no active review threads that need resolution - the previous comments were informational nitpick suggestions that don't require explicit resolution.

I'm now approving the PR as requested. ✅

✅ Actions performed

Comments resolved.

rgsl888prabhu · 2025-10-21T15:50:31Z

@CodeRabbit check

coderabbitai · 2025-10-21T15:51:05Z

@rgsl888prabhu: I'll review the changes in this PR for you.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

rgsl888prabhu · 2025-10-21T19:30:40Z

/merge

rgsl888prabhu added 2 commits October 21, 2025 10:04

disable few tests

2169777

fix style

3b890c2

rgsl888prabhu requested a review from a team as a code owner October 21, 2025 15:08

rgsl888prabhu requested review from Iroy30 and removed request for a team October 21, 2025 15:08

rgsl888prabhu self-assigned this Oct 21, 2025

rgsl888prabhu added bug Something isn't working non-breaking Introduces a non-breaking change labels Oct 21, 2025

coderabbitai bot reviewed Oct 21, 2025

View reviewed changes

fix issues

4fb9cbf

coderabbitai bot reviewed Oct 21, 2025

View reviewed changes

fix issues

c50950d

coderabbitai bot reviewed Oct 21, 2025

View reviewed changes

disbale all barrier tests

0c8550b

tmckayus approved these changes Oct 21, 2025

View reviewed changes

rapids-bot bot merged commit 213bf1a into NVIDIA:branch-25.12 Oct 21, 2025
174 of 175 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Disable flaky barrier tests #520

Disable flaky barrier tests #520

Uh oh!

rgsl888prabhu commented Oct 21, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 21, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

rgsl888prabhu commented Oct 21, 2025

Uh oh!

coderabbitai bot commented Oct 21, 2025

Uh oh!

rgsl888prabhu commented Oct 21, 2025

Uh oh!

coderabbitai bot commented Oct 21, 2025

Uh oh!

rgsl888prabhu commented Oct 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Disable flaky barrier tests #520

Disable flaky barrier tests #520

Uh oh!

Conversation

rgsl888prabhu commented Oct 21, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue

Checklist

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

rgsl888prabhu commented Oct 21, 2025

Uh oh!

coderabbitai bot commented Oct 21, 2025

Uh oh!

rgsl888prabhu commented Oct 21, 2025

Uh oh!

coderabbitai bot commented Oct 21, 2025

Uh oh!

rgsl888prabhu commented Oct 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rgsl888prabhu commented Oct 21, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 21, 2025 •

edited

Loading