Summary: | Add support for test262 JavaScriptCore tests | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||||||
Component: | Tools / Tests | Assignee: | Aakash Jain <aakash_jain> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aakash_jain, ap, buildbot, caitp, clopez, commit-queue, dbates, fpizlo, joepeck, keith_miller, lforschler, saam, ysuzuki | ||||||||
Priority: | P2 | ||||||||||
Version: | Other | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=170711 https://bugs.webkit.org/show_bug.cgi?id=171578 https://bugs.webkit.org/show_bug.cgi?id=182552 |
||||||||||
Attachments: |
|
Description
Aakash Jain
2017-04-05 15:27:41 PDT
Created attachment 306339 [details]
Proposed patch
Attachment 306339 [details] did not pass style-queue:
ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:146: [RunTest262TestsTest.assertResults] Undefined variable 'RunTest262Tests' [pylint/E0602] [5]
ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:155: [RunTest262TestsTest.test_no_regressions_output] Undefined variable 'SUCCESS' [pylint/E0602] [5]
ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:159: [RunTest262TestsTest.test_failure_output] Undefined variable 'FAILURE' [pylint/E0602] [5]
ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:166: [RunTest262TestsTest.test_failures_output] Undefined variable 'FAILURE' [pylint/E0602] [5]
Total errors found: 4 in 4 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 306339 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=306339&action=review Looks sane to me. > Tools/ChangeLog:12 > + (RunTest262Tests.countFailures): method ot count the failures. method => Method ot => to > Tools/ChangeLog:13 > + (Test262Factory): Added Test262 Factory class. Factory => factory > Tools/ChangeLog:16 > + (RunTest262TestsTest.assertResults): helper method. helper => Helper > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:337 > + logText = cmd.logs['stdio'].getText() ' => " There are a series of patches that gets test262 expectation failures down to 0. All the changes are in commit-queue now so should be landed by tomorrow so this bot can start out green. Comment on attachment 306339 [details]
Proposed patch
Awesome, this is very cool. Thanks for getting everything working Aakash and Joe!
Created attachment 306458 [details]
Updated patch
Changed from Sierra to El Capitan since the available bots have el capitan.
Fixed typos in ChangeLog.
(In reply to Aakash Jain from comment #6) > Created attachment 306458 [details] > Updated patch > > Changed from Sierra to El Capitan since the available bots have el Why not just update the bots to Sierra? Don't we control the bots? > Why not just update the bots to Sierra? Don't we control the bots?
These are Xserves, which are not supported in macOS Sierra.
Created attachment 306523 [details]
Updated patch
Added "Reviewed by".
Comment on attachment 306523 [details] Updated patch Clearing flags on attachment: 306523 Committed r215108: <http://trac.webkit.org/changeset/215108> All reviewed patches have been landed. Closing bug. working fine: https://build.webkit.org/dashboard/ I'm thinking in adding this step also for the GTK+ bots so they also run this test. However.. I'm wondering why you added new bots that only run this tests instead of making the current bots that run JSC tests run this by simply adding the test path to this list https://trac.webkit.org/browser/trunk/Tools/Scripts/run-javascriptcore-tests?rev=215108#L268 ??? Its because of the time it takes to run to not slow down the current bots, or there is any other reason? Due to this I have also checked that there are other two tests that the bots are not currently running: JSTests/heapProfiler.yaml and JSTests/asyncFunctionTests.yaml Is there any reason to not run this two tests on the bots? (In reply to Carlos Alberto Lopez Perez from comment #13) > I'm thinking in adding this step also for the GTK+ bots so they also run > this test. > > However.. I'm wondering why you added new bots that only run this tests > instead of making the current bots that run JSC tests run this by simply > adding the test path to this list > https://trac.webkit.org/browser/trunk/Tools/Scripts/run-javascriptcore- > tests?rev=215108#L268 ??? > Its because of the time it takes to run to not slow down the current bots, > or there is any other reason? I actually don't know if there was a good reason for it. I just know that there were 2 machines available, so we used them for this. So its kind of a bonus that it doesn't make any of the other bots slow down. These tests are rather quick though, running in 9-10 minutes on the bots per commit. > Due to this I have also checked that there are other two tests that the bots > are not currently running: JSTests/heapProfiler.yaml and > JSTests/asyncFunctionTests.yaml > > Is there any reason to not run this two tests on the bots? We should probably add these to the normal JavaScriptCore tests bot. It seems like an oversight. Please file a new bugzilla bug. (CC me and Aakash!) Not making regular tests slower was the reason for using a separate queue indeed. (In reply to Carlos Alberto Lopez Perez from comment #13) > I'm thinking in adding this step also for the GTK+ bots so they also run > this test. > > However.. I'm wondering why you added new bots that only run this tests > instead of making the current bots that run JSC tests run this by simply > adding the test path to this list > https://trac.webkit.org/browser/trunk/Tools/Scripts/run-javascriptcore- > tests?rev=215108#L268 ??? > Its because of the time it takes to run to not slow down the current bots, > or there is any other reason? > > > Due to this I have also checked that there are other two tests that the bots > are not currently running: JSTests/heapProfiler.yaml and > JSTests/asyncFunctionTests.yaml > > Is there any reason to not run this two tests on the bots? asyncFunctionTests.yaml was only there for convenience before shipping. Those should be covered by the stress test suite now, and IIRC they are. I will send a patch to clean it up tomorrow if nobody else does. (In reply to Joseph Pecoraro from comment #14) > (In reply to Carlos Alberto Lopez Perez from comment #13) > > I'm thinking in adding this step also for the GTK+ bots so they also run > > this test. > > > > However.. I'm wondering why you added new bots that only run this tests > > instead of making the current bots that run JSC tests run this by simply > > adding the test path to this list > > https://trac.webkit.org/browser/trunk/Tools/Scripts/run-javascriptcore- > > tests?rev=215108#L268 ??? > > Its because of the time it takes to run to not slow down the current bots, > > or there is any other reason? > > I actually don't know if there was a good reason for it. I just know that > there were 2 machines available, so we used them for this. I think you can add more workers to any slave. So you can have N machines for the JSC tester. Check for example the Capitan Release WK2 tester: https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29 ^^ It has 2 buildslaves connected (at the right-top you can see: bot208 and bot209) > So its kind of a > bonus that it doesn't make any of the other bots slow down. These tests are > rather quick though, running in 9-10 minutes on the bots per commit. > > But this complicates things from an infrastructure point of view. Adding a new test to a current set of tests makes it automatically work on all platforms and bots. Adding a new type of bot requires new hardware and new deployments. It also has to be done by each one of the port/bot admins. In the GTK+ bot case it will be much easier for me to make the current bots run this extra step rather than adding new bots. However, if you still want to keep this test262 bots, then I guess I can do this change only for the GTK+/JSCOnly ports: that is to add the test262 step for the JSC bot only if platform is GTK+ or JSCOnly. On the other hand, if you think it will be a good idea to merge test262 inside run-javascript-tests then things can be simplified quite a lot. > > Due to this I have also checked that there are other two tests that the bots > > are not currently running: JSTests/heapProfiler.yaml and > > JSTests/asyncFunctionTests.yaml > > > > Is there any reason to not run this two tests on the bots? > > We should probably add these to the normal JavaScriptCore tests bot. It > seems like an oversight. Please file a new bugzilla bug. (CC me and Aakash!) Filled: https://bugs.webkit.org/show_bug.cgi?id=171578 |