| Summary: | EWS emails about build failure should include relevant error logs | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||||
| Component: | Tools / Tests | Assignee: | Aakash Jain <aakash_jain> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ap, jbedard, rackler, ryanhaddad, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | Other | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=203418 https://bugs.webkit.org/show_bug.cgi?id=215264 https://bugs.webkit.org/show_bug.cgi?id=215330 https://bugs.webkit.org/show_bug.cgi?id=216566 |
||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 215220 | ||||||||
| Attachments: |
|
||||||||
|
Description
Aakash Jain
2020-08-05 10:44:07 PDT
Created attachment 406012 [details]
Patch
Comment on attachment 406012 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406012&action=review > Tools/BuildSlaveSupport/ews-build/steps.py:-1398 > - return defer.succeed(None) Was this line removed intentionally? > Tools/BuildSlaveSupport/ews-build/steps.py:1408 > + def getResults(self, name): What is the return value of this function? I ask because it looks like @defer.inlineCallbacks is overriding that. > Tools/BuildSlaveSupport/ews-build/steps.py:1411 > + defer.returnValue(None) This looks like an early exit, but I'm not familiar with this 'defer' idiom, do we not need to return so we exit early? Comment on attachment 406012 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406012&action=review >> Tools/BuildSlaveSupport/ews-build/steps.py:-1398 >> - return defer.succeed(None) > > Was this line removed intentionally? Yes, it was moved. This line belonged to start() method earlier, and now also it belongs to start() method. >> Tools/BuildSlaveSupport/ews-build/steps.py:1411 >> + defer.returnValue(None) > > This looks like an early exit, but I'm not familiar with this 'defer' idiom, do we not need to return so we exit early? Yes. Also this method is mostly copied from AnalyzeAPITestsResults::getTestsResults() Created attachment 406026 [details]
Patch
Committed r265302: <https://trac.webkit.org/changeset/265302> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406026 [details]. |