Conversation
📝 WalkthroughWalkthroughParameterized test cases in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@python/cuopt/cuopt/tests/linear_programming/test_python_API.py`:
- Around line 515-525: The unconditional skips on the pytest.param entries for
the barrier-augmented configurations (the pytest.param with keys CUOPT_FOLDING,
CUOPT_DUALIZE, CUOPT_ORDERING, CUOPT_AUGMENTED and the other similar
pytest.param blocks) must be made trackable and time-bounded: replace the bare
pytest.mark.skip(...) with a skip reason that includes a tracker/issue ID (e.g.
"CUOPT-XXXXX") and a clear re-enable condition ("re-enable when barrier
initial-point fix is in the build"), add a TODO comment referencing the issue ID
next to the pytest.param, and (where appropriate) consider using
pytest.mark.xfail or pytest.skipif with a short expiry date or env-flag to avoid
permanent loss of numerical-correctness coverage; also ensure the tests validate
numerical correctness of the optimization results rather than only running
without error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 47726764-8795-42ce-bb01-2286bed18e31
📒 Files selected for processing (1)
python/cuopt/cuopt/tests/linear_programming/test_python_API.py
| pytest.param( | ||
| "mixed", | ||
| { | ||
| CUOPT_FOLDING: 1, | ||
| CUOPT_DUALIZE: 0, | ||
| CUOPT_ORDERING: -1, | ||
| CUOPT_AUGMENTED: 1, | ||
| }, | ||
| marks=pytest.mark.skip( | ||
| reason="Barrier augmented-system numerical issue; re-enable when barrier initial-point fix is in the build" | ||
| ), |
There was a problem hiding this comment.
Avoid indefinite untracked skips for barrier coverage.
At Line 515, Line 563, and Line 624, these cases are now unconditionally skipped, which drops numerical-correctness coverage for key barrier configurations. Please make the disablement explicitly time-bounded/trackable (issue ID + re-enable condition) so this does not become permanent technical debt.
Suggested change
+_BARRIER_TEMP_DISABLE_MARK = pytest.mark.xfail(
+ reason=(
+ "TODO(#<issue-id>): Barrier augmented-system numerical issue. "
+ "Re-enable after barrier initial-point fix lands in build."
+ ),
+ run=False,
+)
+
pytest.param(
"mixed",
{
CUOPT_FOLDING: 1,
CUOPT_DUALIZE: 0,
CUOPT_ORDERING: -1,
CUOPT_AUGMENTED: 1,
},
- marks=pytest.mark.skip(
- reason="Barrier augmented-system numerical issue; re-enable when barrier initial-point fix is in the build"
- ),
+ marks=_BARRIER_TEMP_DISABLE_MARK,
),
@@
pytest.param(
"augmented_system",
{
CUOPT_AUGMENTED: 1,
},
- marks=pytest.mark.skip(
- reason="Barrier augmented-system numerical issue; re-enable when barrier initial-point fix is in the build"
- ),
+ marks=_BARRIER_TEMP_DISABLE_MARK,
),
@@
pytest.param(
"combo3_with_dual_init",
{
CUOPT_AUGMENTED: 1,
CUOPT_BARRIER_DUAL_INITIAL_POINT: 1,
CUOPT_ELIMINATE_DENSE_COLUMNS: True,
},
- marks=pytest.mark.skip(
- reason="Barrier augmented-system numerical issue; re-enable when barrier initial-point fix is in the build"
- ),
+ marks=_BARRIER_TEMP_DISABLE_MARK,
),Also applies to: 563-570, 624-633
🤖 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_python_API.py` around lines
515 - 525, The unconditional skips on the pytest.param entries for the
barrier-augmented configurations (the pytest.param with keys CUOPT_FOLDING,
CUOPT_DUALIZE, CUOPT_ORDERING, CUOPT_AUGMENTED and the other similar
pytest.param blocks) must be made trackable and time-bounded: replace the bare
pytest.mark.skip(...) with a skip reason that includes a tracker/issue ID (e.g.
"CUOPT-XXXXX") and a clear re-enable condition ("re-enable when barrier
initial-point fix is in the build"), add a TODO comment referencing the issue ID
next to the pytest.param, and (where appropriate) consider using
pytest.mark.xfail or pytest.skipif with a short expiry date or env-flag to avoid
permanent loss of numerical-correctness coverage; also ensure the tests validate
numerical correctness of the optimization results rather than only running
without error.
Description
These tests are flaky and are disabled, and these will be moved to C layer later.
Checklist