Bug 161816 – Fix mastercfg_unittest

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jonathan Bedard]

Changes brought on by https://bugs.webkit.org/show_bug.cgi?id=161699 and increased debugging capabilities on build-bots.

[Jonathan Bedard]

Created attachment 288443 [details]
Patch

[WebKit Commit Bot]

Attachment 288443 [details] did not pass style-queue:


ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:368:  whitespace before ':'  [pep8/E203] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jonathan Bedard]

Note that EWS is incorrect to flag the ':' in this case (based on the format of this and other files).  Also note that given the circumstances we use --no-sample-on-timeout, it fits better in config.json, especially since the future intent is to run --no-sample-on-timeout for all platforms.

[Jonathan Bedard]

Comment on attachment 288443 [details]
Patch

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

> Tools/BuildSlaveSupport/build.webkit.org-config/config.json:-212
> -                      "triggers": ["ios-simulator-9-release-tests-wk1", "ios-simulator-9-release-tests-wk2"],

This is definitely wrong, will be updated shortly

[Daniel Bates]

Comment on attachment 288443 [details]
Patch

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

> Tools/ChangeLog:3
> +        Fix mastercfg_unittest, move --no-sample-on-timeout to config.json

r-, please separate out the fixed in this patch into separate bugs. This patch contains at least three bug fixes: 1) fix the unit tests (*) 2) Use additionalArguments to pass "--no-sample-on-timeout" to the test runner instead of hardcoding this behavior with respect to the bot "Apple iOS 9 Simulator Release WK2 (Tests)" and 3) change the triggers for "Apple iOS 9 Simulator Release (Build)" (though you implies this change may be incorrect). It is preferred that each bug/patch contains one fix. This allows each bug to serve a forum for a single fix making more straightforward to follow the conversation and iterate on a fix. It also has the advantage that if there is bug in one part of a patch that we do not need to revert the entire patch instead we can revert the bad part (since it will be a patch associated with its own bug).

(*) I thought we were going to do this before we would land the patch for bug #161699 based on our in-person conversation on 09/07; I am saddened that we did not.

[Jonathan Bedard]

(In reply to comment #5)
> Comment on attachment 288443 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=288443&action=review
> 
> ...
>

Based on what you have said, I think that this patch needs to be 2 bugs, not 3.  The trigger change which I noted was a typo on my part which I noticed earlier today and quickly made note of instead of uploading a change.

I had intended to separate the unit tests from bug #161699 because I originally attempted to test build-bot properties, until coming to the conclusion that this would have required more infrastructure than would be worth it, and included the small unit test fix in this patch.  Sorry I didn't communicate that plan well!

[Jonathan Bedard]

Created attachment 288742 [details]
Patch

[Jonathan Bedard]

This new patch addresses only the fixes to the master config unit tests.

[WebKit Commit Bot]

Attachment 288742 [details] did not pass style-queue:


ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:368:  whitespace before ':'  [pep8/E203] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jonathan Bedard]

Created attachment 288745 [details]
Patch

[WebKit Commit Bot]

Attachment 288745 [details] did not pass style-queue:


ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:368:  whitespace before ':'  [pep8/E203] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Daniel Bates]

Comment on attachment 288745 [details]
Patch

Thank you for keeping this bug focused on fixing the unit tests and for fixing the unit tests.

[WebKit Commit Bot]

Comment on attachment 288745 [details]
Patch

Clearing flags on attachment: 288745

Committed r205904: <http://trac.webkit.org/changeset/205904>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.