Bug 177723 - EWS bot should fail if a new test is missing its result
Summary: EWS bot should fail if a new test is missing its result
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
: 181940 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-10-01 12:44 PDT by Sam Weinig
Modified: 2020-10-15 09:43 PDT (History)
9 users (show)

See Also:


Attachments
Patch (3.99 KB, patch)
2020-10-15 06:49 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2017-10-01 12:44:42 PDT
I found it quite surprising that the EWS bot didn't fail when I updated a change with test missing. I would expect that to also be a failure condition.
Comment 1 Radar WebKit Bug Importer 2017-10-02 09:31:40 PDT
<rdar://problem/34768404>
Comment 2 Alexey Proskuryakov 2018-01-22 13:25:09 PST
*** Bug 181940 has been marked as a duplicate of this bug. ***
Comment 3 Aakash Jain 2020-10-15 06:37:31 PDT
Noticed this in: https://ews-build.webkit.org/#/builders/24/builds/28039. Patch 411263 added two tests with missing results. layout-tests step indicated '2 missing results' but did not mark the step/build as failed.

The fix is to simply mark the build step as failed, that would cause layout-tests to be retried, and then retry on clean build, and since clean build would pass, it would cause the build to be marked as failed.
Comment 4 Aakash Jain 2020-10-15 06:49:10 PDT
Created attachment 411436 [details]
Patch
Comment 5 Aakash Jain 2020-10-15 06:50:04 PDT
Comment on attachment 411436 [details]
Patch

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

> Tools/BuildSlaveSupport/ews-build/steps.py:2000
> +                elif line.find('missing results') >= 0:

Added this elif explicitly for readability. Can remove this as well if reviewers feel strongly about it.
Comment 6 Jonathan Bedard 2020-10-15 08:52:59 PDT
Comment on attachment 411436 [details]
Patch

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

>> Tools/BuildSlaveSupport/ews-build/steps.py:2000
>> +                elif line.find('missing results') >= 0:
> 
> Added this elif explicitly for readability. Can remove this as well if reviewers feel strongly about it.

I'm fine with the elif, but it seems like the failure condition should be first. (since in the event a line contained both 'flakes' and 'missing results', we would want to mark the build as failed). I know this concern is more about purity than practicality at the moment, though.
Comment 7 Aakash Jain 2020-10-15 09:15:57 PDT
(In reply to Jonathan Bedard from comment #6)
> since in the event a line contained both 'flakes' and 'missing results'
'flakes' and 'missing results' words aren't being checked literally, they store output of regular expressions matching from https://trac.webkit.org/browser/webkit/trunk/Tools/BuildSlaveSupport/ews-build/steps.py#L1949
Comment 8 EWS 2020-10-15 09:27:53 PDT
Committed r268526: <https://trac.webkit.org/changeset/268526>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 411436 [details].
Comment 9 Aakash Jain 2020-10-15 09:43:28 PDT
Restarted EWS server to pick up this change.