Bug 208521 – commit-queue should update working directory and reapply patch just before commiting

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Aakash Jain]

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.

[Aakash Jain]

Created attachment 392293 [details]
Patch

[Jonathan Bedard]

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?

[Aakash Jain]

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

[Aakash Jain]

Committed r257803: <https://trac.webkit.org/changeset/257803>

[Radar WebKit Bug Importer]

<rdar://problem/60012903>

[Jonathan Bedard]

(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.

[Alexey Proskuryakov]

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.

[Aakash Jain]

(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.