Skip to content

Fix benchmark regression checks for throughput metrics - #454

Open
Shubham-Padkonde wants to merge 1 commit into
amazon-ion:masterfrom
Shubham-Padkonde:fix/benchmark-throughput-comparison
Open

Shubham-Padkonde wants to merge 1 commit into
amazon-ion:masterfrom
Shubham-Padkonde:fix/benchmark-throughput-comparison

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown

Issue #, if available: Fixes #281

Description of changes:

The benchmark compare --fail command currently treats an increase in operations per second as a regression and lets a throughput drop pass. Use the report field's direction of improvement when deciding whether the threshold was exceeded. Keep the signed percentage in the report, and preserve the existing comparison for fields without an explicit direction.

Added CLI regressions for mean/min/max throughput in both directions, threshold boundaries, and the existing time/file-size behavior. The test subprocess now uses sys.executable so it runs in the same environment as pytest.

Validation: all 79 benchmark CLI/spec tests pass on Python 3.9.25. Six new throughput cases fail against the original comparison logic and pass with the fix.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Benchmark CLI compare can have inaccurate results for some metrics

1 participant