Adding individual status code statistics table #347
Conversation
There was a problem hiding this comment.
There's a room for an overall improvement here. Status codes are already being aggregated, but they're aggregated by "index": 2xx, 3xx, ..., which is less granular than what you're trying to achieve with this change. Instead of creating an additional implementation which does the same but in a more granular way, why don't you make the existing implementation more granular and use the finer grained granularity when renderStatusCode is true, and fallback to the existing behavior when it's false?
| if (!statusCodeStats[statusCode]) { | ||
| statusCodeStats[statusCode] = { count: 1 } | ||
| } else { | ||
| statusCodeStats[statusCode].count++ | ||
| } |
There was a problem hiding this comment.
is there any reason to not use the object values as the count, instead of having an object with a single property count? It would be more consistent with how the other stats are aggregated
There was a problem hiding this comment.
@simoneb , you are absolutely right, there are many ways this could be improved. I was not certain of how far I can mod the code and still remain within the original author's intent. My goal was additive change only without impacting any of the existing functionality. If you believe that you can take this further, please do.
There was a problem hiding this comment.
As far as object with a single property, I envision that there may be other attributes aggregated about each individual statusCode, other than a count, and thus wanted to provide a harness for that. An example could be request timing average per code.
There was a problem hiding this comment.
@maxfortun on the overall approach let's hear what @mcollina thinks. On the count property, if it's not necessary, I'd say avoid it
|
@mcollina unit test added |
| opts = autocannon.parseArguments(args) | ||
| } | ||
|
|
||
| const resultStr = autocannon.printResult(exampleResult, opts) |
There was a problem hiding this comment.
I'm not sure this change to the tests is actually being used anywhere. Can you please confirm?
There was a problem hiding this comment.
Looks like I forgot to add a file.
Fixed. Thanks.
#346
Add
--renderStatusCodesflag to render individual status code statistics table