Summary: | Use a regex to check if a test step is for JavaScriptCore | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Srinivasan Vijayaraghavan <webkit> | ||||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Srinivasan Vijayaraghavan
2016-06-28 13:51:02 PDT
Created attachment 282279 [details]
Patch
Comment on attachment 282279 [details]
Patch
r=me
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. (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. /(?=.*test)(?=.*jsc)/ or /(?=.*test)(?=.*jsc)/ would work, as would using two regexes. Will upload after testing locally. (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. Created attachment 282285 [details]
Patch
Nope, one more change. Created attachment 282286 [details]
Patch
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). Created attachment 282288 [details]
Patch
(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. Comment on attachment 282288 [details] Patch Clearing flags on attachment: 282288 Committed r202606: <http://trac.webkit.org/changeset/202606> All reviewed patches have been landed. Closing bug. |