[webkit-reviews] review granted: [Bug 232529] Move detection of flaky JSC tests to run-jsc-stress-tests : [Attachment 443676] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Nov 19 17:37:02 PST 2021

Jonathan Bedard <jbedard at apple.com> has granted Angelos Oikonomopoulos
<angelos at igalia.com>'s request for review:
Bug 232529: Move detection of flaky JSC tests to run-jsc-stress-tests

Attachment 443676: Patch


--- Comment #25 from Jonathan Bedard <jbedard at apple.com> ---
Comment on attachment 443676
  --> https://bugs.webkit.org/attachment.cgi?id=443676

View in context: https://bugs.webkit.org/attachment.cgi?id=443676&action=review

I touched base with Angelos about this patch today. With most Apple folks out
next week and no JSC folks commenting, I don't think it makes sense to continue
blocking this patch. I'm not a Ruby expert, but looking at this patch, combined
with EWS and the fact that it shouldn't change behavior until builedbot is
deployed, I'm r+ing with the expectation that Angelos will land Monday, Nov
22nd and handle any fallout next week.

> Tools/Scripts/run-jsc-stress-tests:620
> +	   @flaky = flaky

This name doesn't indicate well what expect "flaky" to be. I'm assuming it's
the Flaky object? Might be good to indicate that in the initializer by calling
it flakyObject. I think it's fine to keep "flaky" as is in the actual class.

> Tools/Scripts/run-jsc-stress-tests:845
> +class Flaky

Flaky seems like the wrong name for this object. It looks to like it's more
accurately "Tolerance" or "RetryParameters". If we rename it, we should also
rename any member variables which contain it.

More information about the webkit-reviews mailing list