WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
159971
Bots should run built-ins generator tests
https://bugs.webkit.org/show_bug.cgi?id=159971
Summary
Bots should run built-ins generator tests
youenn fablet
Reported
2016-07-20 06:43:13 PDT
Bots should run built-ins generator tests
Attachments
Patch
(10.21 KB, patch)
2016-07-20 06:47 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Adding an extra step for build bots
(23.26 KB, patch)
2016-07-20 09:02 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Updated according review
(23.90 KB, patch)
2016-07-20 09:43 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
youenn fablet
Comment 1
2016-07-20 06:47:06 PDT
Created
attachment 284100
[details]
Patch
Alexey Proskuryakov
Comment 2
2016-07-20 08:05:00 PDT
Comment on
attachment 284100
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=284100&action=review
> Tools/ChangeLog:15 > + * Scripts/webkitpy/tool/steps/runtests.py:
It doesn't seem right to unconditionally tie these tests to run-webkit-tests. Tools that automatically process results won't know what to do with these failures.
youenn fablet
Comment 3
2016-07-20 08:48:06 PDT
> It doesn't seem right to unconditionally tie these tests to > run-webkit-tests. Tools that automatically process results won't know what > to do with these failures.
What additional logic is needed to actually process failing results? I guess it may be more appropriate to add an additional step dedicated to built-in tests for build bots.
youenn fablet
Comment 4
2016-07-20 09:02:25 PDT
Created
attachment 284108
[details]
Adding an extra step for build bots
WebKit Commit Bot
Comment 5
2016-07-20 09:04:54 PDT
Attachment 284108
[details]
did not pass style-queue: ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:368: whitespace before ':' [pep8/E203] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Alexey Proskuryakov
Comment 6
2016-07-20 09:27:12 PDT
Comment on
attachment 284108
[details]
Adding an extra step for build bots View in context:
https://bugs.webkit.org/attachment.cgi?id=284108&action=review
Please add this step to ProductiveSteps in BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotIteration.js, otherwise the dashboard will not process failure correctly. These tests should also run on EWS. Not something for this patch, however it is important for EWS to catch issues that can make actual bots red.
> Tools/ChangeLog:16 > + Bots should run built-ins generator tests > +
https://bugs.webkit.org/show_bug.cgi?id=159971
Duplicate ChangeLog.
> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:567 > + name = "builtins-generation-tests" > + description = ["builtins-tests running"] > + descriptionDone = ["builtins-tests"] > + command = ["python", "./Tools/Scripts/run-builtins-generator-tests"]
These names are all different (builtins-generation-tests, builtins-generator-tests, builtins-tests). Can a single name be used? I suggest matching script name, as that's the easiest.
Alexey Proskuryakov
Comment 7
2016-07-20 09:27:57 PDT
This is almost an r+, however the patch is small enough and there are enough suggested changes that I'd like to take a look at the final version before landing.
youenn fablet
Comment 8
2016-07-20 09:43:29 PDT
Created
attachment 284113
[details]
Updated according review
WebKit Commit Bot
Comment 9
2016-07-20 09:45:30 PDT
Attachment 284113
[details]
did not pass style-queue: ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:368: whitespace before ':' [pep8/E203] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
youenn fablet
Comment 10
2016-07-20 09:46:31 PDT
Thanks for the review.
>
https://bugs.webkit.org/attachment.cgi?id=284108&action=review
> > Please add this step to ProductiveSteps in > BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/ > BuildbotIteration.js, otherwise the dashboard will not process failure > correctly.
OK
> These tests should also run on EWS. Not something for this patch, however it > is important for EWS to catch issues that can make actual bots red.
Agreed, that was the purpose of the previous patch. how should it be done then?
> > Tools/ChangeLog:16 > > + Bots should run built-ins generator tests > > +
https://bugs.webkit.org/show_bug.cgi?id=159971
> > Duplicate ChangeLog.
Fixed.
> > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:567 > > + name = "builtins-generation-tests" > > + description = ["builtins-tests running"] > > + descriptionDone = ["builtins-tests"] > > + command = ["python", "./Tools/Scripts/run-builtins-generator-tests"] > > These names are all different (builtins-generation-tests, > builtins-generator-tests, builtins-tests). Can a single name be used? > > I suggest matching script name, as that's the easiest.
Fixed.
Alexey Proskuryakov
Comment 11
2016-07-20 09:55:47 PDT
Comment on
attachment 284113
[details]
Updated according review
> Agreed, that was the purpose of the previous patch. how should it be done then?
I think that EWS need to explicitly know about the various tests, so that it could report failures intelligently. For internal tool tests like this one, it is also very likely that any failures will be cross-platform, so we'll need to add logic to coalesce Bugzilla updates.
Blaze Burg
Comment 12
2016-07-20 09:59:00 PDT
(In reply to
comment #11
)
> Comment on
attachment 284113
[details]
> Updated according review > > > Agreed, that was the purpose of the previous patch. how should it be done then? > > I think that EWS need to explicitly know about the various tests, so that it > could report failures intelligently. For internal tool tests like this one, > it is also very likely that any failures will be cross-platform, so we'll > need to add logic to coalesce Bugzilla updates.
Is there an example where this has been done already for EWS? There are many similar internal test suites (bindings tests, generator tests, etc) that will need to be hooked in the same way. So an example would be multiply-useful.
Alexey Proskuryakov
Comment 13
2016-07-20 10:24:01 PDT
There was some work started in
bug 140744
.
WebKit Commit Bot
Comment 14
2016-07-20 23:10:54 PDT
Comment on
attachment 284113
[details]
Updated according review Clearing flags on attachment: 284113 Committed
r203489
: <
http://trac.webkit.org/changeset/203489
>
WebKit Commit Bot
Comment 15
2016-07-20 23:10:58 PDT
All reviewed patches have been landed. Closing bug.
youenn fablet
Comment 16
2016-07-21 03:22:36 PDT
Patch landed but builtin tests are not ruined. Is there any additional thing to do?
Alexey Proskuryakov
Comment 17
2016-07-21 08:48:14 PDT
I do not know if a manual master restart is needed. Lucas would know.
Csaba Osztrogonác
Comment 18
2016-08-25 09:19:05 PDT
(In reply to
comment #16
)
> Patch landed but builtin tests are not ruined. > Is there any additional thing to do?
(In reply to
comment #17
)
> I do not know if a manual master restart is needed. Lucas would know.
After a month bots started to run these tests and they fail on EFL/GTK bots.
Csaba Osztrogonác
Comment 19
2016-08-25 09:38:17 PDT
filed a new bug report to track Linux failures:
bug161196
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug