fix: reject empty name/ticker/description in launch command#10
fix: reject empty name/ticker/description in launch command#10P-r-e-m-i-u-m wants to merge 4 commits intochainstacklabs:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds input validation to the CLI launch command to reject empty or whitespace-only Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
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.
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 `@tests/test_commands/test_launch_cmd.py`:
- Around line 8-53: Consolidate the six near-duplicate tests
(test_launch_empty_name, test_launch_whitespace_name, test_launch_empty_ticker,
test_launch_whitespace_ticker, test_launch_empty_desc,
test_launch_whitespace_desc) into a single parametrized pytest function that
iterates over tuples of (flag, value, expected_error_substring) and calls
runner.invoke(app, ["launch", flag_key_map[flag], value, ...]) or constructs the
args accordingly; use monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path)) once
per test, assert non-zero exit_code and that expected_error_substring is in
result.output.lower(); reference the existing runner.invoke call site and the
CLI flags "--name", "--ticker", "--desc" to locate where to build the param
list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f079dfd3-642a-4c6a-946d-e82b2803b324
📒 Files selected for processing (2)
src/pumpfun_cli/commands/launch.pytests/test_commands/test_launch_cmd.py
|
"Refactored tests to use @pytest.mark.parametrize as suggested. Ready for re-review @smypmsa |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/test_commands/test_launch_cmd.py (1)
17-22:⚠️ Potential issue | 🟠 MajorStrengthen this test to prevent false positives and enforce no-network behavior.
This case currently passes if any error includes the keyword, and it does not explicitly guard against RPC execution. Assert the exact validation message and mock/guard RPC entrypoints so the test fails if network paths are reached.
Suggested patch
+from unittest.mock import Mock import pytest from typer.testing import CliRunner from pumpfun_cli.cli import app @@ def test_launch_rejects_empty_inputs(tmp_path, monkeypatch, name, ticker, desc, expected_error): """launch rejects empty or whitespace-only name, ticker, or description.""" monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path)) + launch_token_mock = Mock(side_effect=AssertionError("launch_token must not be called for invalid input")) + monkeypatch.setattr("pumpfun_cli.commands.launch.launch_token", launch_token_mock) + result = runner.invoke(app, ["launch", "--name", name, "--ticker", ticker, "--desc", desc]) assert result.exit_code != 0 - assert expected_error in result.output.lower() + assert f"token {expected_error} cannot be empty." in result.output.lower() + launch_token_mock.assert_not_called()As per coding guidelines, "Tests must use mocks for RPC calls, no real network."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_commands/test_launch_cmd.py` around lines 17 - 22, The test test_launch_rejects_empty_inputs should assert the exact validation message and ensure no network/RPC code is executed: use monkeypatch to replace the RPC entrypoint(s) the CLI uses (the module/method the launch command calls; e.g., the RPC client or caller used by app/launch) with a stub that raises AssertionError if invoked, invoke runner.invoke(app, ["launch", ...]) as before, assert result.exit_code != 0, and assert the normalized output equals the exact expected_error string (e.g., result.output.strip().lower() == expected_error) so the test fails on wrong messages or if RPC paths are reached. Include the test function name test_launch_rejects_empty_inputs and the caller runner.invoke(app, ...) to locate where to add these monkeypatches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/test_commands/test_launch_cmd.py`:
- Around line 17-22: The test test_launch_rejects_empty_inputs should assert the
exact validation message and ensure no network/RPC code is executed: use
monkeypatch to replace the RPC entrypoint(s) the CLI uses (the module/method the
launch command calls; e.g., the RPC client or caller used by app/launch) with a
stub that raises AssertionError if invoked, invoke runner.invoke(app, ["launch",
...]) as before, assert result.exit_code != 0, and assert the normalized output
equals the exact expected_error string (e.g., result.output.strip().lower() ==
expected_error) so the test fails on wrong messages or if RPC paths are reached.
Include the test function name test_launch_rejects_empty_inputs and the caller
runner.invoke(app, ...) to locate where to add these monkeypatches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cca55149-ef14-4128-a885-dd04fae19d40
📒 Files selected for processing (1)
tests/test_commands/test_launch_cmd.py
|
"Mocked launch_token to prevent RPC calls in tests and tightened assertions to exact error messages. Ready for re-review @smypmsa |
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 `@tests/test_commands/test_launch_cmd.py`:
- Around line 11-18: Update the pytest parametrization to use a tuple literal
for the argument names in the pytest.mark.parametrize decorator (replace the
comma-delimited string "name,ticker,desc,expected_error" with a tuple like
("name","ticker","desc","expected_error")) in the test function in
tests/test_commands/test_launch_cmd.py so it satisfies PT006 and leaves the
parameter value list unchanged; the change should be made where
pytest.mark.parametrize is used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 66ff7c32-8cd2-44fd-83fa-05821e4cad30
📒 Files selected for processing (1)
tests/test_commands/test_launch_cmd.py
|
"Fixed PT006 — changed parametrize string to tuple form. Ready for re-review @smypmsa |
Signed-off-by: 🄂ʏᴇᴅ 🄰ʙᴅᴜʟ 🄰ᴍᴀ🄝 ✧ <amanbaba9404522@gmail.com>
Signed-off-by: 🄂ʏᴇᴅ 🄰ʙᴅᴜʟ 🄰ᴍᴀ🄝 ✧ <amanbaba9404522@gmail.com>
Signed-off-by: 🄂ʏᴇᴅ 🄰ʙᴅᴜʟ 🄰ᴍᴀ🄝 ✧ <amanbaba9404522@gmail.com>
Signed-off-by: 🄂ʏᴇᴅ 🄰ʙᴅᴜʟ 🄰ᴍᴀ🄝 ✧ <amanbaba9404522@gmail.com>
7f5a318 to
42466dd
Compare
|
"Rebased onto latest main. Ready for merge @smypmsa |
Closes #5
Added validation in
launch.pyto reject empty or whitespace-only values for--name,--ticker, and--descbefore any RPC or IPFS calls are made.Changes
src/pumpfun_cli/commands/launch.pyname,ticker, anddescafterstate = ctx.objtests/test_commands/test_launch_cmd.pyWhat changed
Security / funds / transaction implications
Architecture / layering
Tests