Skip to content

Mark L1 tests as disabled - #1126

Merged
ramakrishnap-nv merged 2 commits into
NVIDIA:mainfrom
mlubin:l1
Apr 22, 2026
Merged

Mark L1 tests as disabled#1126
ramakrishnap-nv merged 2 commits into
NVIDIA:mainfrom
mlubin:l1

Conversation

@mlubin

@mlubin mlubin commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

We don't run these tests, and agents sometimes forget the -E L1TEST instructions. Let's mark them as disabled.

@mlubin
mlubin requested review from a team as code owners April 21, 2026 00:34
@mlubin
mlubin requested a review from ramakrishnap-nv April 21, 2026 00:34
@copy-pr-bot

copy-pr-bot Bot commented Apr 21, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@mlubin mlubin added non-breaking Introduces a non-breaking change improvement Improves an existing functionality labels Apr 21, 2026
@mlubin

mlubin commented Apr 21, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 561a496

@coderabbitai

coderabbitai Bot commented Apr 21, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 793e35f1-c7df-4d98-b687-68d1779cbd4d

📥 Commits

Reviewing files that changed from the base of the PR and between 561a496 and 592b77a.

📒 Files selected for processing (2)
  • CONTRIBUTING.md
  • cpp/tests/routing/CMakeLists.txt
🚧 Files skipped from review as they are similar to previous changes (1)
  • cpp/tests/routing/CMakeLists.txt

📝 Walkthrough

Walkthrough

Documentation updated to remove the L1-test exclusion from the ctest command; CMake test configuration now marks two L1 test targets as disabled so they won't run in standard CTest executions.

Changes

Cohort / File(s) Summary
Documentation
CONTRIBUTING.md
Replaced the previous ctest ... -E L1TEST exclusion with ctest --test-dir ${CUOPT_HOME}/cpp/build and removed the sentence explaining that L1 tests are excluded because they are expensive and not run during typical development.
Test Configuration
cpp/tests/routing/CMakeLists.txt
Added set_tests_properties(...) to set DISABLED TRUE for RETAIL_L1TEST and ROUTING_L1TEST, preventing those tests from running in standard CTest runs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: marking L1 tests as disabled, which matches the core objective of the pull request.
Description check ✅ Passed The description is directly related to the changeset, explaining the rationale for marking L1 tests as disabled and why this change is needed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@mlubin

mlubin commented Apr 21, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 592b77a

@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator

/ok to test 4def18a

@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator

/merge

@ramakrishnap-nv
ramakrishnap-nv merged commit 9ca72e8 into NVIDIA:main Apr 22, 2026
102 of 106 checks passed
@mlubin
mlubin deleted the l1 branch April 22, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants