fix(benchmark): avoid negative curl/cargo cases that fail the benchmark job - #3430
Open
ousamabenyounes wants to merge 1 commit into
Open
fix(benchmark): avoid negative curl/cargo cases that fail the benchmark job#3430ousamabenyounes wants to merge 1 commit into
ousamabenyounes wants to merge 1 commit into
Conversation
Contributor
|
Hey @ousamabenyounes , thanks for this, actually @KuSh already opened an issue on mockhttp repo and this should be solved now that it is merged , jaredwray/mockhttp#181 , so apparently we should get a stable /json/1 |
Collaborator
|
Still missing a release on mockhttp side for the stable endpoint to be available though |
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
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.
Problem
The shared
benchmarkCI job (scripts/benchmark.sh) fails withBENCHMARK FAILED: 1 filter(s) produced more tokens than raw output. The offending case iscurl json, which hitshttps://mockhttp.org/json/1— a response so tiny (a handful of tokens) that RTK's curl formatting adds a few tokens rather than saving any (35 → 38, a negative case). Because this lives in the shared benchmark script, every branch based ondevelopinherits the red job, regardless of its own diff.A second latent negative existed on the cargo side:
cargo buildon a trivial project (only aFinishedline) could render a summary larger than the raw output.Fix
scripts/benchmark.sh: replace the micromockhttp.org/json/1fixture with a small local JSON payload big enough to exercise real formatting savings, and point thewgetcase at a stable path.src/cmds/rust/cargo_cmd.rs: guard both the streamed and non-streamed cargo build success summaries withcore::guard::never_worse(raw, summary)so RTK never emits a summary larger than the raw output when the input is already tiny.Test verification (RED → GREEN)
Two new unit tests assert the raw output is preserved when the summary would be larger:
RED (before the
never_worseguard): the cargo build success path returned the rendered summary unconditionally, which is larger than the raw single-lineFinished ...input.GREEN (after):
assert_eq!(result, output)— raw output is returned unchanged.Full local suite on this branch (off
develop):