This bug is meant to help debug https://bugs.webkit.org/show_bug.cgi?id=161696. Spindumps are large, so they are only being selectively enabled.
Created attachment 288162 [details] Patch
Comment on attachment 288162 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288162&action=review > Tools/ChangeLog:3 > + Run spindumps on timeout on iOS 9 WK 2release simulators. "iOS 9 WK 2release simulators" => "iOS 9 Simulator Release WK2 (Tests) bots" > Tools/ChangeLog:9 > + (RunWebKitTests): Disable sample on timeout for everything except iOS 9 WK2 release simulators. Do we know how much more time this may introduce for a test cycle? > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:350 > + if WithProperties("%(buildername)s") != "Apple iOS 9 Simulator Release WK2 (Tests)": > + command.append("--no-sample-on-timeout") Does this work? From my understanding the values of Buildbot properties are not available until start() is called. Also, please add a test for this change. The unit tests for Buildbot are in file <https://trac.webkit.org/browser/trunk/Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py>. You can run the tests by invoking "python mastercfg_unittest.py" inside directory Tools/BuildSlaveSupport/build.webkit.org-config.
Comment on attachment 288162 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288162&action=review >> Tools/ChangeLog:9 >> + (RunWebKitTests): Disable sample on timeout for everything except iOS 9 WK2 release simulators. > > Do we know how much more time this may introduce for a test cycle? The issue won't be time (it's about 30 seconds per timeout test, so basically doubling the timeout time) it will be memory >> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:350 >> + command.append("--no-sample-on-timeout") > > Does this work? From my understanding the values of Buildbot properties are not available until start() is called. Also, please add a test for this change. The unit tests for Buildbot are in file <https://trac.webkit.org/browser/trunk/Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py>. You can run the tests by invoking "python mastercfg_unittest.py" inside directory Tools/BuildSlaveSupport/build.webkit.org-config. Well, I'm parsing the builder-name instead of the platform. From my understanding, the builder-name is defined in the constructor, after all, we actually use the builder name in the constructor already (line 341). I used the builder-name instead on the platform name since this change is specifically targeting a builder.
Created attachment 288190 [details] Patch
Adding Unit Tests for this requires infrastructure we currently do not have. Local testing has confirmed this patch, and a later patch will include Unit Tests for RunWebKitTests more generally.
Comment on attachment 288190 [details] Patch Seems like concerns have been addressed, rs=me
Comment on attachment 288190 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288190&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:360 > + if WithProperties("%(buildername)s") != "Apple iOS 9 Simulator Release WK2 (Tests)": Is there a way we can do this an avoid hardcoding some or all of the name of this bot? I mean, we will setup bots for iOS 10 soon. On another note, it seems excessive to interpolate the value of a Buildbot property using WithProperties() as a means to gets its value. I suggest we use self.getProperty(X) to get the value directly.
Comment on attachment 288190 [details] Patch Clearing flags on attachment: 288190 Committed r205573: <http://trac.webkit.org/changeset/205573>
All reviewed patches have been landed. Closing bug.
Comment on attachment 288190 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288190&action=review >> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:360 >> + if WithProperties("%(buildername)s") != "Apple iOS 9 Simulator Release WK2 (Tests)": > > Is there a way we can do this an avoid hardcoding some or all of the name of this bot? I mean, we will setup bots for iOS 10 soon. > > On another note, it seems excessive to interpolate the value of a Buildbot property using WithProperties() as a means to gets its value. I suggest we use self.getProperty(X) to get the value directly. I'm not sure how long we intend to keep this change. Since it is designed to catch a specific problem, I think for the time being, hardcoding is appropriate. If we determine that these logs are useful more generally, we can use something more general. Looking closer, yes, self.getProperty is probably a better choice and will likely be used when I update this code-path for testing.