Repository navigation
Display the configured percentile in the console reports - #490
Conversation
The console formatters matched `percentiles: %{99 => value}` and
hardcoded the "99th %" column, so a `percentiles` configuration
without 99 raised a MatchError and lost all results.
Report the highest calculated percentile instead, which keeps 99 as
the default while making the column follow the configuration.
The fourth argument of format_scenario carries the percentile now, so the branches matching true could never be reached - dialyzer flagged both.
PragTob
left a comment
There was a problem hiding this comment.
Thanks a lot for your contribution! 💚
I have some questions around the main implementation which I left inline of code where I don't quite understand why it is the way it is. Would be glad to figure that out 👀
Otherwise looks good and thanks for contributing! I'm shocked this was broken actually. If you're comfortable, it'd be great to add a test to benchee_test.exs - which is where the more higher level tests live. Just configuring a different percentile and doing broad assumptions that it all works out (the file has plenty of examples). I just wanna make sure there is no other "I hard expect 99th percentile to be there" bug anywhere in the stack :)
|
Addressed the implementation questions in the latest commit:
The two helper regressions fail against the previous PR helper and pass with the update. Native Ubuntu/WSL Elixir 1.14 validation: full suite passes (138 doctests, 352 tests, 4 excluded, 1 skipped), affected-file formatting passes, test compilation with warnings-as-errors passes, and strict Credo reports no issues. Credo notes some checks are incompatible with this local Elixir version; the hosted version/platform matrix remains pending. |
|
Thank you! |
Console reports previously required the 99th percentile and raised a
MatchErrorwhen a supportedpercentilesconfiguration omitted it. All three formatters now report the highest calculated percentile and label the column accordingly. The default remains 99; an empty percentile configuration uses the calculated 75th percentile. Reports without percentile results displayN/A, while missing expected ranks still raise instead of being silently hidden.Fixes #489.
Validation after the review revisions: real
Benchee.runregressions cover[50, 90]and[]across all three report headers. The two helper regressions fail against the previous implementation. On Ubuntu with Elixir 1.14/Erlang, the full suite passed: 138 doctests, 352 tests, 0 failures, 4 excluded and 1 skipped. Formatting, test compilation with warnings treated as errors and strict Credo also passed. This does not establish the upstream Elixir 1.20/OTP 29 matrix; no current-head CI checks are reported.