Bug 159224 – Use a regex to check if a test step is for JavaScriptCore

• Component: New Bugs
• Status: RESOLVED FIXED

[Srinivasan Vijayaraghavan]

Use a regex to check if a test step is for JavaScriptCore

[Srinivasan Vijayaraghavan]

Created attachment 282279 [details]
Patch

[Geoffrey Garen]

Comment on attachment 282279 [details]
Patch

r=me

[Alexey Proskuryakov]

Comment on attachment 282279 [details]
Patch

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

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotCombinedQueueView.js:112
> -                    } else if (mostRecentFinishedIteration.failedTestSteps.length === 1 && ["jscore-test", "webkit-32bit-jsc-test", "webkit-jsc-cloop-test"].indexOf(mostRecentFinishedIteration.failedTestSteps[0].name) >= 0) {
> +                    } else if (mostRecentFinishedIteration.failedTestSteps.length === 1 && /jsc/.test(mostRecentFinishedIteration.failedTestSteps[0].name) === true) {

Would it make sense to make sure that "test" is in the name too? That would decrease the chances of something like "build-jsc" or "configure-jsc-dependencies" to confuse the logic.

[Srinivasan Vijayaraghavan]

(In reply to comment #3)
> Would it make sense to make sure that "test" is in the name too? That would
> decrease the chances of something like "build-jsc" or
> "configure-jsc-dependencies" to confuse the logic.

Agreed, working on that now.

[Srinivasan Vijayaraghavan]

/(?=.*test)(?=.*jsc)/ or /(?=.*test)(?=.*jsc)/ would work, as would using two regexes. Will upload after testing locally.

[Srinivasan Vijayaraghavan]

(In reply to comment #5)
> /(?=.*test)(?=.*jsc)/ or /(?=.*test)(?=.*jsc)/ would work, as would using
> two regexes. Will upload after testing locally.

Meant to say /(?=.*test)(?=.*jsc)/ or /(?=.*jsc)(?=.*test)/, since they are equivalent.

[Srinivasan Vijayaraghavan]

Created attachment 282285 [details]
Patch

[Srinivasan Vijayaraghavan]

Nope, one more change.

[Srinivasan Vijayaraghavan]

Created attachment 282286 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 282286 [details]
Patch

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

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotIteration.js:254
> +            else if (/(?=.*test)(?=.*jsc)/.test(step.name) === true)

WebKit coding style calls for no comparison to true, should be just if (/(?=.*test)(?=.*jsc)/.test(step.name))

Sorry for missing it the first time around (I blame Geoff of course).

[Srinivasan Vijayaraghavan]

Created attachment 282288 [details]
Patch

[Srinivasan Vijayaraghavan]

(In reply to comment #10)
> Comment on attachment 282286 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=282286&action=review
> 
> > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotIteration.js:254
> > +            else if (/(?=.*test)(?=.*jsc)/.test(step.name) === true)
> 
> WebKit coding style calls for no comparison to true, should be just if
> (/(?=.*test)(?=.*jsc)/.test(step.name))
> 
> Sorry for missing it the first time around (I blame Geoff of course).

Sorry about that, just resubmitted.

[WebKit Commit Bot]

Comment on attachment 282288 [details]
Patch

Clearing flags on attachment: 282288

Committed r202606: <http://trac.webkit.org/changeset/202606>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.