[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]

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
https://bugs.webkit.org/show_bug.cgi?id=232529

Attachment 443676: Patch

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




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

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.