fix(benchmark): reject a malformed annotation instead of ignoring it#885
Merged
Conversation
Each marker is extracted with a numeric sed pattern and fell back to the default when it did not match, so a malformed value was indistinguishable from an absent annotation: @revs=abc ran 1 revolution @its=abc ran 1 iteration @max_ms=abc dropped the threshold, so it could never fail all reporting success while measuring something other than what was written. A marker that is present but yielded no value is now an error; an absent annotation still takes its default. The call site captured into `read <<<"$(...)"`, which discards the command substitution's status, so it now captures into a variable first. Closes #884
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤔 Background
Related #884
Each benchmark marker is extracted with a numeric
sedpattern and fell back to thedefault when it did not match, so a malformed value was indistinguishable from an absent
annotation — the benchmark ran with different parameters than the ones written, and
reported success:
@revs=abc@its=abc@max_ms=abcSame class as #871/#873/#875/#877/#879, at the annotation layer rather than the CLI.
💡 Changes
read -r ... <<<"$(parse_annotations ...)"discards the command substitution's exit status, so the failure could not propagate@revs=5 @its=2still applies correctly)