-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed baseline compare issues #5605
Fixed baseline compare issues #5605
Conversation
I think CI is failing here for unrelated reasons ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have another test that checks if outputs are sorted?
dd70917
to
c90993c
Compare
Generated by 🚫 Danger |
I've updated the test so that there are multiple violations in the output of compare, and I'm passing the initial violations in in reverse order, to verify the sorting of the output. |
c90993c
to
ae39cec
Compare
ae39cec
to
bf56623
Compare
Fixes #5606 where
baseline compare
can produce some false alarms, because we throw away thetext
information from one of the baselines, but the files on disk may be absent or have changed, so the newtext
values read duringfilter
ing may (likely) not be appropriate.In this PR, the logic and public API for
filter
is completely unchanged, but the public function converts theStyleViolations
toBaselineViolations
, and retrieves any existingBaselineViolations
for the file.These
BaselineViolations
are then passed to a privatefilter
function, which implements the corefilter
ing logic.The
compare
function now passes theBaselineViolations
from each Baseline to this private function.We now also sort the violations reported by
baseline compare
, by location and rule, so the output should now be wholly deterministic which is was not quite before.