Skip to content

fix: print usage on unexpected arguments in lavinmqperf#1796

Merged
kickster97 merged 2 commits intomainfrom
lavinmqperf-unexpected-arg-usage
Mar 10, 2026
Merged

fix: print usage on unexpected arguments in lavinmqperf#1796
kickster97 merged 2 commits intomainfrom
lavinmqperf-unexpected-arg-usage

Conversation

@oskgu360
Copy link
Member

@oskgu360 oskgu360 commented Mar 5, 2026

WHAT is this pull request doing?

Abort run if unexpected arguments are provided.

Stupid users (me) need help. I got confused why stuff worked when it defaulted to localhost behind the scenes, but real cause of error was that I forgot to pass option flags :)

HOW can this pull request be tested?

Prior (connecting to default localhost)

lavinmqperf throughput amqp://nouser:nopw@nodomain.com:5672/test
Publish rate: 1519814 msgs/s Consume rate: 1519778 msgs/s
...

After

bin/lavinmqperf throughput amqp://nouser:nopw@nodomain.com:5672/test
Unexpected argument: amqp://nouser:nopw@nodomain.com:5672/test

Usage: bin/lavinmqperf [protocol] [throughput | bind-churn | queue-churn | connection-churn | connection-count | queue-count] [arguments]
    -h, --help                       Show this help
....

@oskgu360 oskgu360 requested a review from a team as a code owner March 5, 2026 09:18
@claude
Copy link

claude bot commented Mar 5, 2026

PR Review: No issues found. The change correctly uses abort to report unexpected arguments remaining after OptionParser#parse, which mutates args in place to remove recognized flags.

Copy link
Member

@kickster97 kickster97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to me!

@spuun
Copy link
Member

spuun commented Mar 9, 2026

What about using OptionsParser#unknown_args and place it in Perf? I think that's doable, that "argumetns" are never used so any argument is an unknown argument?

@kickster97 kickster97 merged commit f4fa7ce into main Mar 10, 2026
17 of 18 checks passed
@kickster97 kickster97 deleted the lavinmqperf-unexpected-arg-usage branch March 10, 2026 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants