Add check for missing --name or --all options + integration test#53
Add check for missing --name or --all options + integration test#53mikusaq wants to merge 1 commit into
Conversation
Add integration tests for this edge case.
📝 WalkthroughWalkthroughThe PR adds CLI argument validation to enforce that ChangesCLI Validation Enhancement
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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/integration_tests/test_invalid_arguments.py (1)
93-94: ⚡ Quick winAssert the specific validation message, not just generic
"ERROR"Line 93 and Line 109 only verify a generic failure marker, so these tests can pass for unrelated errors. Assert the new contract message (
requires either --name or --all) to lock this behavior.Proposed test hardening
def test_06_build_package_without_name_or_all(test_image, packager_binary, context, test_repo): @@ - assert "ERROR" in stdout + assert "ERROR" in stdout + assert "build-package requires either --name or --all" in stdout @@ def test_07_build_app_without_name_or_all(test_image, packager_binary, context, test_repo): @@ - assert "ERROR" in stdout + assert "ERROR" in stdout + assert "build-app requires either --name or --all" in stdoutAlso applies to: 109-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration_tests/test_invalid_arguments.py` around lines 93 - 94, Replace the weak generic assertions that check for "ERROR" in stdout with a precise assertion for the validation message; update the assertions that currently read assert "ERROR" in stdout to assert "requires either --name or --all" in stdout (apply to both occurrences that reference stdout) so the tests verify the exact contract message rather than a generic error marker.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/integration_tests/test_invalid_arguments.py`:
- Around line 93-94: Replace the weak generic assertions that check for "ERROR"
in stdout with a precise assertion for the validation message; update the
assertions that currently read assert "ERROR" in stdout to assert "requires
either --name or --all" in stdout (apply to both occurrences that reference
stdout) so the tests verify the exact contract message rather than a generic
error marker.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae2aeb9c-55ae-4e0b-8aa4-8c403740f2c3
📒 Files selected for processing (2)
cmd/bap-builder/CmdArgs.gotests/integration_tests/test_invalid_arguments.py
Summary by CodeRabbit
Bug Fixes
build-packageandbuild-appcommands.--nameor--allflags to be specified.Tests