Disable incumbent test for neos test case#1023
Merged
rapids-bot[bot] merged 1 commit intoNVIDIA:release/26.04from Apr 1, 2026
Merged
Disable incumbent test for neos test case#1023rapids-bot[bot] merged 1 commit intoNVIDIA:release/26.04from
rapids-bot[bot] merged 1 commit intoNVIDIA:release/26.04from
Conversation
rgsl888prabhu
added
non-breaking
Iroy30
approved these changes
Apr 1, 2026
📝 WalkthroughWalkthroughMPS dataset test cases in incumbent callback tests were commented out due to PDLP concurrent/incumbent callback behavior issues. Two parametrized test entries across two test functions were disabled. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
python/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.py (2)
104-112: Consider using@pytest.mark.skipinstead of empty parameter list.An empty
@pytest.mark.parametrizelist causes pytest to collect the test with 0 items, which can be confusing in test reports. Using explicit skip markers provides clearer visibility into why tests are disabled and ensures they appear in skip reports.♻️ Suggested approach using skip markers
`@pytest.mark.parametrize`( "file_name", [ - # ("/mip/swath1.mps"), # Disabled: https://github.com/NVIDIA/cuopt/issues/967 (PDLP concurrent / incumbent callbacks). - # ("/mip/neos5-free-bound.mps"), # Disabled: https://github.com/NVIDIA/cuopt/issues/967 (PDLP concurrent / incumbent callbacks). + pytest.param("/mip/swath1.mps", marks=pytest.mark.skip(reason="https://github.com/NVIDIA/cuopt/issues/967 - PDLP concurrent / incumbent callbacks")), + pytest.param("/mip/neos5-free-bound.mps", marks=pytest.mark.skip(reason="https://github.com/NVIDIA/cuopt/issues/967 - PDLP concurrent / incumbent callbacks")), ], ) def test_incumbent_get_callback(file_name):This approach:
- Makes skipped tests visible in pytest reports with clear reasons
- Keeps test parameters intact for easy re-enablement when issue
#967is resolved- Follows pytest best practices for temporarily disabling tests
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.py` around lines 104 - 112, The parametrize decorator on test_incumbent_get_callback currently provides an empty parameter list which yields 0 collected items; replace the empty `@pytest.mark.parametrize`("file_name", [...]) with a pytest.mark.skip(reason="Disabled due to https://github.com/NVIDIA/cuopt/issues/967 (PDLP concurrent / incumbent callbacks)") applied to the test function (e.g., `@pytest.mark.skip`(...)\ndef test_incumbent_get_callback(file_name):) so the test is explicitly reported as skipped while preserving the commented file entries for easy re-enablement; keep the call to _run_incumbent_solver_callback(file_name, include_set_callback=False) unchanged.
115-123: Same recommendation applies here.Use
pytest.param(..., marks=pytest.mark.skip(...))for consistent skip reporting across both test functions.♻️ Suggested approach
`@pytest.mark.parametrize`( "file_name", [ - # ("/mip/swath1.mps"), # Disabled: https://github.com/NVIDIA/cuopt/issues/967 (PDLP concurrent / incumbent callbacks). - # ("/mip/neos5-free-bound.mps"), # Disabled: https://github.com/NVIDIA/cuopt/issues/967 (PDLP concurrent / incumbent callbacks). + pytest.param("/mip/swath1.mps", marks=pytest.mark.skip(reason="https://github.com/NVIDIA/cuopt/issues/967 - PDLP concurrent / incumbent callbacks")), + pytest.param("/mip/neos5-free-bound.mps", marks=pytest.mark.skip(reason="https://github.com/NVIDIA/cuopt/issues/967 - PDLP concurrent / incumbent callbacks")), ], ) def test_incumbent_get_set_callback(file_name):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.py` around lines 115 - 123, The parameterized skip should use pytest.param with marks to report skipped cases consistently: update the pytest.mark.parametrize list in test_incumbent_get_set_callback to replace the commented-out file entries with pytest.param("/mip/swath1.mps", marks=pytest.mark.skip(reason="PDLP concurrent / incumbent callbacks: https://github.com/NVIDIA/cuopt/issues/967")) and similarly for "/mip/neos5-free-bound.mps"; ensure you keep the same parameter name ("file_name") and that the test function test_incumbent_get_set_callback still calls _run_incumbent_solver_callback(file_name, include_set_callback=True).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@python/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.py`:
- Around line 104-112: The parametrize decorator on test_incumbent_get_callback
currently provides an empty parameter list which yields 0 collected items;
replace the empty `@pytest.mark.parametrize`("file_name", [...]) with a
pytest.mark.skip(reason="Disabled due to
https://github.com/NVIDIA/cuopt/issues/967 (PDLP concurrent / incumbent
callbacks)") applied to the test function (e.g., `@pytest.mark.skip`(...)\ndef
test_incumbent_get_callback(file_name):) so the test is explicitly reported as
skipped while preserving the commented file entries for easy re-enablement; keep
the call to _run_incumbent_solver_callback(file_name,
include_set_callback=False) unchanged.
- Around line 115-123: The parameterized skip should use pytest.param with marks
to report skipped cases consistently: update the pytest.mark.parametrize list in
test_incumbent_get_set_callback to replace the commented-out file entries with
pytest.param("/mip/swath1.mps", marks=pytest.mark.skip(reason="PDLP concurrent /
incumbent callbacks: https://github.com/NVIDIA/cuopt/issues/967")) and similarly
for "/mip/neos5-free-bound.mps"; ensure you keep the same parameter name
("file_name") and that the test function test_incumbent_get_set_callback still
calls _run_incumbent_solver_callback(file_name, include_set_callback=True).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b5c85e6a-2230-4481-a63e-2d486acb61b7
📒 Files selected for processing (1)
python/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.py
Collaborator
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Disabling these test cases as there are flaky and causing segfaults, and issue has been created to follow-up #967
Checklist