search

tenox7 / ttyplot

a realtime plotting utility for terminal/console with data input from stdin
Apache License 2.0
957 stars 42 forks source link

Add options to stresstest.c #116

Closed edgar-bonet closed 7 months ago

edgar-bonet commented 7 months ago

Draft pull request

This pull request adds option parsing, and multiple options to torture.c, which are intended to exercise corner cases in ttyplot:

$ ./torture -h
Usage: ./torture [-h] [-2] [-c] [-g] [-r rate]
  -h       print this help message and exit
  -2       output two waves
  -c       randomly chunk the output
  -g       occasionally output garbage
  -r rate  sample rate in samples/s (default: 100)

This PR is marked as “draft” because it is intended to be merged in a future “development” branch, that integrates previous pull requests. A such, it sits on top of my branch “next”, which merges PRs #98, #99, #106, #110 and #114. This PR proper is four commits: see the diff.

hartwork commented 7 months ago

@edgar-bonet I'm a bit worried that rebasing this into latest development with #118 merged now coulld kill my review comments above due to the rename. Maybe we can complete the pull request in isolation with the original filename and only rebase with a rename when it's done, to be safe; just an idea.

edgar-bonet commented 7 months ago

All your comments make a lot of sense to me. Applied them all, thanks for the good suggestions. I also restored \n (instead of space) as a separator, as I had no good reason for changing it.

I also added a second commit to align the help message with your PR #120.

I am OK with completing this review round before rebasing on development.

edgar-bonet commented 7 months ago

I integrated your last two comment that, while minor, were in my eyes relevant. Thanks for that! I also marked the PR as ready: I had labeled it “draft” only waiting for the rebase that would integrate the name change → stresstest.c. Should I do that rebase now?

hartwork commented 7 months ago

I integrated your last two comment that, while minor, were in my eyes relevant. Thanks for that! I also marked the PR as ready: I had labeled it “draft” only waiting for the rebase that would integrate the name change → stresstest.c. Should I do that rebase now?

@edgar-bonet sounds good, please go ahead, but the use of bool still seems to be missing while being resolve, did you forget to push something maybe?

hartwork commented 7 months ago

@edgar-bonet nevermind, maybe my reload was broken, I see bool now, thanks! :+1:

edgar-bonet commented 7 months ago

Rebased on development: now it's ready, the changed file is now stresstest.c.

hartwork commented 7 months ago

@edgar-bonet thanks, let's get this merged! :smiley: