Skip to content

fix(compare): gate regressions on the binary under test - #287

Open
BryanFRD wants to merge 2 commits into
mainfrom
fix/284-gate-on-binary-under-test
Open

BryanFRD wants to merge 2 commits into
mainfrom
fix/284-gate-on-binary-under-test

Conversation

@BryanFRD

Copy link
Copy Markdown
Contributor

The relative check now only considers -ferrflow-binary- keys, so npm and docker (published release) timings no longer fail a PR. ferrflow_binary_size_mb is now the size of the ferrflow binary on PATH (the one under test) instead of the npm-published package.

Adds a compare.bats case with regressing npm/docker keys that must not fail. bats is not installed locally, so the new case was not run here; the equivalent script run exited 0.

Closes #284

@ferrfleet-interne ferrfleet-interne Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. The relative gate now covers only -ferrflow-binary- keys, which matches the existing absolute thresholds, and the new bats case covers npm/docker being ignored. The binary size now comes from the binary on PATH, the same one being timed, not a freshly downloaded npm package. One nit inline.

Expect one benign size delta on the first run against an old baseline: du -m rounded up to whole MiB, while the new value is exact.

Comment thread scripts/run.sh Outdated

@ferrfleet-interne ferrfleet-interne Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stat -L nit is fixed in 58715be. Nothing else has changed since my last approval, and I have nothing new to raise.

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.

fix(compare): gate regressions on the binary under test, not the published package

1 participant