tests(smoke): fix failureReasonsMask flake - #11791
Conversation
patrickhulce
left a comment
There was a problem hiding this comment.
I was gonna ask about the regex because it feels like the more straightforward fix, but I suppose it is weird to coerce numbers.
LGTM nice find!
|
Thanks for taking care of this! Might be worth investigating why the test became flaky all of a sudden. |
yah. it only happens on ToT and not stable, so presumably its a commit that landed in the past day or so. |
|
Found the culprit: https://chromium.googlesource.com/chromium/src/+/f0946b3ebc528e5eb4a1a9c033357d1df7a436b6 Looks like |
|
@adamraine nice find! did you bisect or just realize this landed at the right time to explain things?
sgtm! yah i can see it happening again. (partially thanks to you! 😝 ) |
Bisect. |
Example failure:
Most of the
smoke_3_ToTfailures ive been seeing this afternoon are coming from this assertion.The animation reliably gets
kUnsupportedCSSProperty(1 << 13) === 8192. (which is what we want)Sometimes it also gets
kTargetHasInvalidCompositingState(1 << 5) == 32. (but not always) Together they sum to 8224.@adamraine and I have investigated
kTargetHasInvalidCompositingStatea few times and it's not actionable, in part because it shows up under very hard-to-predict circumstances.. Basically its presence is flakyso this PR changes the smoketest expectation to handle it being there and not.
on the impl: i stated with /(8192)|(8224)/ but our report-assert doesn't expect to apply regexes to numbers, and I didn't want to deal with that. Overall i'm happier with this anyway.