Bug 223011 - [build.webkit.org] Update RunJavaScriptCoreTests step for new buildbot
Summary: [build.webkit.org] Update RunJavaScriptCoreTests step for new buildbot
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
Depends on:
Blocks:
 
Reported: 2021-03-09 17:32 PST by Aakash Jain
Modified: 2021-03-10 07:33 PST (History)
6 users (show)

See Also:


Attachments
Patch (5.20 KB, patch)
2021-03-09 17:38 PST, 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 Aakash Jain 2021-03-09 17:32:15 PST
We have upgraded build.webkit.org to latest Buildbot. We should update RunJavaScriptCoreTests as per new Buildbot, e.g.: use logobserver instead of cmd.logs['stdio'].getText()
Comment 1 Aakash Jain 2021-03-09 17:38:38 PST
Created attachment 422785 [details]
Patch
Comment 2 dewei_zhu 2021-03-09 18:10:21 PST
Comment on attachment 422785 [details]
Patch

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

> Tools/CISupport/build-webkit-org/steps.py:437
> +        self.failedTestCount = 0

Is this used anywhere in this change?
Comment 3 Aakash Jain 2021-03-09 18:26:33 PST
Comment on attachment 422785 [details]
Patch

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

>> Tools/CISupport/build-webkit-org/steps.py:437
>> +        self.failedTestCount = 0
> 
> Is this used anywhere in this change?

It is used by the base class TestWithFailureCount (inside getResultSummary())
Comment 4 dewei_zhu 2021-03-09 18:48:11 PST
Comment on attachment 422785 [details]
Patch

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

>>> Tools/CISupport/build-webkit-org/steps.py:437
>>> +        self.failedTestCount = 0
>> 
>> Is this used anywhere in this change?
> 
> It is used by the base class TestWithFailureCount (inside getResultSummary())

Why was it working before this change?  By checking the code, it looks like it will be set by `TestWithFailureCount.commandComplete`.
Anyway, initialize an variable in the here seems no harm.
Comment 5 dewei_zhu 2021-03-09 18:48:23 PST
r=me
Comment 6 Aakash Jain 2021-03-10 04:34:28 PST
Comment on attachment 422785 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422785&action=review
> Why was it working before this change?
This wasn't working properly after the upgrade. logText in countFailures was empty (as deprecated cmd.logs['stdio'].getText() return empty string in new buildbot), and so custom step message wasn't being set. 

It should be set now, e.g.: '2 JSC tests failed' in https://build.webkit-dev.org/#/builders/84/builds/3
Comment 7 EWS 2021-03-10 04:52:39 PST
Committed r274209: <https://commits.webkit.org/r274209>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422785 [details].
Comment 8 Radar WebKit Bug Importer 2021-03-10 04:53:14 PST
<rdar://problem/75260591>
Comment 9 Aakash Jain 2021-03-10 07:33:52 PST
Restated buildbot to pick up this change this morning.

Seems to be working fine. e.g.: https://build.webkit.org/#/builders/102/builds/90 now says: "29 JSC tests failed", while earlier it used to say: "jscore-test (failure)"