Feature burst

[liketoeatcheese]

#276 Introducing burst:
Using --burst-duration: the user can now add a delay between requests. For example: If -n 10 --burst-duration 2s, oha will run one request every 2s until it reaches 10s
Adding along with --burst-requests: the rate of requests at one point in time the user wants to send. E.g: In the example above -n 10 --burst-duration 2s --burst-requests 2, oha will now run 2 requests every 2s

Changes to the code:

# client.rs
# Added logic to accept Burst instead 
pub enum QueryLimit {
    Qps(usize),
    Burst(std::time::Duration, usize),
}

# Functions to accept Bursts instead of qps
work_with_qps()
work_with_qps_latency_correction()
work_until_with_qps()
work_until_with_qps_latency_correction()

# main.rs
# struct Opts: Added `burst_duration` and `burst_requests`
# main function:
if let Some(duration) = opts.duration.take() {
     # Added additional `match` to take in `Burst` as well, not just qps
}

[hatoo]

Thank you! I really appreciate it.

Since I'm busy now, I'll review it sometime this week.

But at a glance,

• I really want to be just one argument --burst because inferring --burst-requests=1 as a default doesn't make sense to me.
• There are tests in tests/tests.rs, this feature is not very suitable to test fully but some degree of test is preferred.
• Could you update README.md (Usage section) and CHANGELOG.md if you have time?

[liketoeatcheese]

No worries :)

Regarding your input:

README.md

Sure thing!

Test suite

I'm not sure how I would test assertion with work_* functions either, as it doesn't return any thing. If you can point out what you would like me to test, I would be happy to do it.

Burst arguments

I tried to implement the argument the way you wanted it, but I couldn't find a way for it to parse in the struct:
--burst {duration} {rate}

Maybe you can give me some guidance. First, I used #[clap()] attribute macro as like so:

    #[clap(
        help = "Introduce delay between a predefined number of requests.
Group: Burst
Note: If qps is specified, burst will be ignored",
        long = "burst"
    )]
    burst_duration: Option<Duration>,
    burst_requests: Option<usize>,

But when I build this, I tested it out with --help. I was getting --burst <BURST_DURATION>, which discarded the burst_requests type. So I was looking for documentation to group these 2 together. First, sorted out to clap structs, and found ArgGroup:

which states:

By placing arguments in a logical group, you can create easier requirement and exclusion rules instead of having to list each argument individually, or when you want a rule to apply “any but not all” arguments.

Now looking out for the derive version of this. I should be able to just do #[group()]. But I couldn't:

I thought it was due to the fact that it's missing some dependencies. So I checked clap doc in feature flags - derive section:

Noticed that it's dependent on the clap_derive crate, which I tried to add on but it still produced the same error. The last thing I tried was adding group method into #arg[] macro attribute, like so:

    #[arg(
        help = "Introduce delay between a predefined number of requests.
Group: Burst
Note: If qps is specified, burst will be ignored",
        long = "burst",
        group = "burst"
    )]
    burst_duration: Option<Duration>,
    #[arg(
        help = "Rates of requests for burst. Default is 1
Group: Burst
Note: If qps is specified, burst will be ignored",
        group = "burst"
    )]
    burst_requests: Option<usize>,

But it failed when I specified both of the arguments. So I finally sorted it out to the way it's currently now.

[hatoo]

Thank you! I leave some reviews

Argment parse

Sure, I've realized it's a current clap's limitation. The implementation is OK for now.
clap-rs/clap#1717

test

Tests are done by starting a test HTTP server and running the command itself using assert_cmd and checking the server's log.

https://github.com/hatoo/oha/blob/master/tests/tests.rs

The tests I want are e.g.

• run oha -n 10 --burst-rate 7 ... and check exactly 10 requests are sent

[liketoeatcheese]

I reviewed and fixed up the comments you made, and also added tests and documentation in README :)

[hatoo]

Great!
I left some nit reviews

note: The CI failure is OK. It seems like a Clippy's bug.

[liketoeatcheese]

Coolio! All fixed :)

[hatoo]

Merged, Thanks! 👍

[liketoeatcheese]

No worries! Thanks for that