208521 2020-03-03 10:24:34 -0800 commit-queue should update working directory and reapply patch just before commiting 2020-03-03 16:13:57 -0800 1 1 1 Unclassified WebKit Tools / Tests Other Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 aakash_jain aakash_jain aakash_jain ap jbedard webkit-bug-importer oldest_to_newest 1625118 0 aakash_jain 2020-03-03 10:24:34 -0800 After applying the patch, commit-queue does building and testing. Building and testing might take some time. Meanwhile there might be multiple commits to the repository. Therefore after running the tests, and just before committing the patch, commit-queue should update the working directory to ToT and re-apply the patch. This is similar to commit-queue behavior in old EWS. 1625119 1 392293 aakash_jain 2020-03-03 10:25:46 -0800 Created attachment 392293 Patch 1625141 2 392293 jbedard 2020-03-03 11:08:29 -0800 Comment on attachment 392293 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=392293&action=review > Tools/BuildSlaveSupport/ews-build/factories.py:222 > self.addStep(RunWebKit1Tests()) Shouldn't this be WebKit2 tests? 1625250 3 392293 aakash_jain 2020-03-03 14:20:33 -0800 Comment on attachment 392293 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=392293&action=review >> Tools/BuildSlaveSupport/ews-build/factories.py:222 >> self.addStep(RunWebKit1Tests()) > > Shouldn't this be WebKit2 tests? commit-queue runs WebKit1 tests. I don't remember the exact reason for that, probably to have coverage on less commonly tested configuration. In old ews commit-queue uses mac port which uses --dump-render-tree https://trac.webkit.org/browser/webkit/trunk/Tools/Scripts/webkitpy/common/config/ports.py#L174 1625251 4 aakash_jain 2020-03-03 14:20:55 -0800 Committed r257803: <https://trac.webkit.org/changeset/257803> 1625252 5 webkit-bug-importer 2020-03-03 14:21:17 -0800 <rdar://problem/60012903> 1625278 6 jbedard 2020-03-03 15:19:44 -0800 (In reply to Aakash Jain from comment #3) > Comment on attachment 392293 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=392293&action=review > > >> Tools/BuildSlaveSupport/ews-build/factories.py:222 > >> self.addStep(RunWebKit1Tests()) > > > > Shouldn't this be WebKit2 tests? > > commit-queue runs WebKit1 tests. I don't remember the exact reason for that, > probably to have coverage on less commonly tested configuration. > In old ews commit-queue uses mac port which uses --dump-render-tree > https://trac.webkit.org/browser/webkit/trunk/Tools/Scripts/webkitpy/common/ > config/ports.py#L174 I think we should revisit the reason. I suspect (although don't know for certain) the original reason is that WebKit2 tests were poorly gardened when they were first enabled. That is no longer the case, and in fact, it is likely WebKit1 tests which are more poorly gardened. 1625300 7 ap 2020-03-03 15:57:45 -0800 IIRC they were gardened equally well, but we knew that WebKit2 ran fewer tests because of unimplemented WebKitTestRunner functionality. That's still true as far as functionality goes, but there are now many WebKit2 only features and more code in WebKit2 itself. So I also think that we should switch CQ to WebKit2. 1625306 8 aakash_jain 2020-03-03 16:13:57 -0800 (In reply to Alexey Proskuryakov from comment #7) > IIRC they were gardened equally well, but we knew that WebKit2 ran fewer > tests because of unimplemented WebKitTestRunner functionality. > > That's still true as far as functionality goes, but there are now many WebKit2 only features and more code in WebKit2 itself. So I also think that we should switch CQ to WebKit2. Switching it in Bug 208544. 392293 2020-03-03 10:25:46 -0800 2020-03-03 11:08:07 -0800 Patch bug-208521-20200303132545.patch text/plain 1236 aakash_jain SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDI1Nzc4NSkKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDEyIEBACisyMDIwLTAzLTAzICBBYWthc2ggSmFpbiAgPGFha2FzaF9qYWluQGFwcGxlLmNv bT4KKworICAgICAgICBjb21taXQtcXVldWUgc2hvdWxkIHVwZGF0ZSB3b3JraW5nIGRpcmVjdG9y eSBhbmQgcmVhcHBseSBwYXRjaCBqdXN0IGJlZm9yZSBjb21taXRpbmcKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIwODUyMQorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogQnVpbGRTbGF2ZVN1cHBvcnQvZXdz LWJ1aWxkL2ZhY3Rvcmllcy5weToKKwogMjAyMC0wMy0wMyAgSml0ZW4gTWVodGEgIDxqbWVodGFA YXBwbGUuY29tPgogCiAgICAgICAgIEFkb3B0IEhUVFAgQWx0ZXJuYXRpdmUgU2VydmljZXMgU3Rv cmFnZQpJbmRleDogVG9vbHMvQnVpbGRTbGF2ZVN1cHBvcnQvZXdzLWJ1aWxkL2ZhY3Rvcmllcy5w eQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBUb29scy9CdWlsZFNsYXZlU3VwcG9ydC9ld3MtYnVpbGQvZmFjdG9y aWVzLnB5CShyZXZpc2lvbiAyNTc3ODUpCisrKyBUb29scy9CdWlsZFNsYXZlU3VwcG9ydC9ld3Mt YnVpbGQvZmFjdG9yaWVzLnB5CSh3b3JraW5nIGNvcHkpCkBAIC0yMjAsMyArMjIwLDcgQEAgY2xh c3MgQ29tbWl0UXVldWVGYWN0b3J5KEZhY3RvcnkpOgogICAgICAgICBzZWxmLmFkZFN0ZXAoS2ls bE9sZFByb2Nlc3NlcygpKQogICAgICAgICBzZWxmLmFkZFN0ZXAoVmFsaWRhdGVQYXRjaChhZGRV UkxzPUZhbHNlLCB2ZXJpZnljcXBsdXM9VHJ1ZSkpCiAgICAgICAgIHNlbGYuYWRkU3RlcChSdW5X ZWJLaXQxVGVzdHMoKSkKKyAgICAgICAgc2VsZi5hZGRTdGVwKFZhbGlkYXRlUGF0Y2goYWRkVVJM cz1GYWxzZSwgdmVyaWZ5Y3FwbHVzPVRydWUpKQorICAgICAgICBzZWxmLmFkZFN0ZXAoQ2hlY2tP dXRTb3VyY2UoKSkKKyAgICAgICAgc2VsZi5hZGRTdGVwKFVwZGF0ZVdvcmtpbmdEaXJlY3Rvcnko KSkKKyAgICAgICAgc2VsZi5hZGRTdGVwKEFwcGx5UGF0Y2goKSkK