[ci] chore: mark nano autoconfig roundtrip as pleasefixme and remove redundant launch script - #3091
Conversation
…redundant launch script The autoconfig_roundtrip tests are already collected by the per-model launch scripts, so the standalone L1_Launch_autoconfig_roundtrip.sh is redundant. Also mark test_nemotron_3_nano_autoconfig_roundtrip with pleasefixme since it is currently broken. Signed-off-by: Chen Cui <chcui@nvidia.com>
📝 WalkthroughWalkthroughA launch script for autoconfig roundtrip tests was deleted, and a test method was marked with a Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/functional_tests/test_groups/models/nemotronh/test_nemotron_h_conversion.py (1)
605-607: Add a comment explaining why this test is markedpleasefixme.Other uses of
@pytest.mark.pleasefixmein this file include explanatory comments (e.g.,# PP=2 broken by hybrid_layer_pattern (PR#2628)). Adding a similar comment here would help future maintainers understand the root cause and when the test can be re-enabled.📝 Suggested change
`@pytest.mark.run_only_on`("GPU") - `@pytest.mark.pleasefixme` + `@pytest.mark.pleasefixme` # TODO: <explain why broken and link to tracking issue> def test_nemotron_3_nano_autoconfig_roundtrip(self, nemotron_3_nano_toy_model_path, tmp_path):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/functional_tests/test_groups/models/nemotronh/test_nemotron_h_conversion.py` around lines 605 - 607, Add an inline comment above the `@pytest.mark.pleasefixme` on the test_nemotron_3_nano_autoconfig_roundtrip function explaining the specific reason this test is marked as pleasefixme (e.g., which bug/PR/issue causes it to fail, relevant failure symptoms, and any conditions like GPU-only), and include a reference ID (issue number or PR) and guidance for when it can be re-enabled so future maintainers understand the root cause and resolution path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@tests/functional_tests/test_groups/models/nemotronh/test_nemotron_h_conversion.py`:
- Around line 605-607: Add an inline comment above the `@pytest.mark.pleasefixme`
on the test_nemotron_3_nano_autoconfig_roundtrip function explaining the
specific reason this test is marked as pleasefixme (e.g., which bug/PR/issue
causes it to fail, relevant failure symptoms, and any conditions like GPU-only),
and include a reference ID (issue number or PR) and guidance for when it can be
re-enabled so future maintainers understand the root cause and resolution path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 39d97732-1309-4ccc-8862-f05d3350edb8
📒 Files selected for processing (2)
tests/functional_tests/launch_scripts/active/L1_Launch_autoconfig_roundtrip.shtests/functional_tests/test_groups/models/nemotronh/test_nemotron_h_conversion.py
💤 Files with no reviewable changes (1)
- tests/functional_tests/launch_scripts/active/L1_Launch_autoconfig_roundtrip.sh
Summary
TestNemotron3NanoConversion::test_nemotron_3_nano_autoconfig_roundtripwith@pytest.mark.pleasefixmesince it is currently broken in the ci environment. THe test runs successfully in the local environment.L1_Launch_autoconfig_roundtrip.shwhich is redundant — the autoconfig roundtrip tests are already collected by the per-model launch scripts.Test plan
Summary by CodeRabbit