Add functionality to run autocannon forever. #94
Conversation
|
Weird, I actually fixed the test issue locally on my machine before pushing, but it is appearing again on CI. investigating |
|
sigh. Nothing happened here. Move along. |
|
Awesome |
| t.end() | ||
| }, 1000) | ||
| } | ||
| }) |
There was a problem hiding this comment.
A callback should be called only once, we should probably move away from the callback, and use an EventEmitter.
There was a problem hiding this comment.
So, this is a weird case, and I only planned on using the event emitter approach. But I realised that this is possible when writing the test so I used it. In the case forever is true, should we throw if a callback is passed in?
There was a problem hiding this comment.
This adds much more complexity when handling options. Have it working, will push so you can review
| instance.on('response', () => { amountProgressBar.tick() }) | ||
| instance.on('reqError', () => { amountProgressBar.tick() }) | ||
| instance.on('done', () => { amountProgressBar.tick(iOpts.amount - 1) }) | ||
| process.once('SIGINT', () => { amountProgressBar.tick(iOpts.amount - 1) }) |
There was a problem hiding this comment.
can we factor this out into a function?
There was a problem hiding this comment.
We could, but it should only be run once, so I left it in the scope of this function to make it clear to readers that this is should only be run once, when the track function is ran.
There was a problem hiding this comment.
I mean, the code above for both amountProgressBar and durationProgressBar looks very similar. we should probably factor it out in its own function, ideally calling the same function with different parameters, or something similar.
(this is a nit of style, nothing more)
There was a problem hiding this comment.
I could curry the callbacks for the events with the relevant progress bar var & amount to tick by (undefined in the case of raw .tick()). Thoughts?
There was a problem hiding this comment.
Okay, so I did the currying thing, and felt super happy with it, and then realised why I had them written like that when it crashed. I use the scope of the progressBar variables to instantiate new ones when restarting the benchmark, but use the same variable reference. By currying, I lose access to the scope-referencing-closure-benefit. I would like to leave this as is, because to get something "cleaner" working would end up adding add much more code, and not necessarily add any benefit. Is that okay with you?
|
If you're happy with this @mcollina, feel free to merge. I don't think I can add anymore to it 👍 |
fixes #92
No perf damage over multiple runs.