-
Notifications
You must be signed in to change notification settings - Fork 10k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert the files in the /test/unit
folder to ES6 modules
#8298
Conversation
/botio unittest |
/botio-linux unittest |
test/unit/jasmine-boot.js
Outdated
@@ -38,33 +38,31 @@ | |||
|
|||
// Modified jasmine's boot.js file to load PDF.js libraries async. | |||
|
|||
'use strict'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual test files are being parsed by SystemJS, which will add a 'use strict';
directive to each module, but unless I'm mistaken this file runs as-is in the browser.
Hence it may be a good idea to keep this line, such that this file still runs in strict mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in the new commit.
test/unit/testreporter.js
Outdated
@@ -1,5 +1,3 @@ | |||
'use strict'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual test files are being parsed by SystemJS, which will add a 'use strict';
directive to each module, but unless I'm mistaken this file runs as-is in the browser.
Hence it may be a good idea to keep this line, such that this file still runs in strict mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in the new commit.
9c9bd08
to
a17bad0
Compare
/botio unittest |
From: Bot.io (Linux)ReceivedCommand cmd_unittest from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/452829772e172f5/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/ce04226cdf08761/output.txt |
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/ce04226cdf08761/output.txt Total script time: 6.86 mins
|
/botio-linux-new unittest |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/0b7a14a7dbd5705/output.txt |
@yurydelendik @brendandahl The Linux bots have some kind of issue with Firefox, but I'm not sure it's related to this patch. Could you check if there are logs or hanging processes? Maybe a reboot would be enough. The unit tests run fine using Firefox on Linux locally, and also on Travis and the Windows bot. |
/botio-linux test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/cc003856ca29ba8/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/cc003856ca29ba8/output.txt Total script time: 31.65 mins
|
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/6065694e41a2ecc/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/dedb42833eeec26/output.txt |
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/dedb42833eeec26/output.txt Total script time: 24.44 mins
|
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/6065694e41a2ecc/output.txt Total script time: 60.00 mins |
Even though the code changes themselves look fine, I cannot help wondering if the slight increase in runtime is somehow connected with the frequent timeouts/issues on the Linux bot that we've seen above? Anyway, despite the diff looking fine, I'll hold off on landing this for now until we're certain that this PR isn't somehow contributing to the Linux issues. |
It looks like Firefox doesn't even start the unit tests at all. Really weird because I run it all the time on Linux without issues. Are the bots doing something special, or are there any logs telling us why Firefox won't even start or what it's doing all the time before it times out? |
/botio-linux test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/6b97f9352f288ce/output.txt |
/botio-linux test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/36be359da818e30/output.txt |
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/36be359da818e30/output.txt Total script time: 60.00 mins |
/botio unittest |
From: Bot.io (Linux)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/03569c2a1376096/output.txt |
I have rebased the patch and resolved merge conflicts. The bots have recently received many updates as well, so we should be good now. |
/botio unittest |
From: Bot.io (Linux)ReceivedCommand cmd_unittest from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/2140866520f00ef/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @timvandermeij received. Current queue size: 1 Live output at: http://54.215.176.217:8877/6f71668c680625b/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/2140866520f00ef/output.txt Total script time: 5.05 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/6f71668c680625b/output.txt Total script time: 6.62 mins
|
/botio unittest |
From: Bot.io (Linux)ReceivedCommand cmd_unittest from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/daea6a63404f0f2/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/903126dbd75dfd3/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/daea6a63404f0f2/output.txt Total script time: 5.18 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/903126dbd75dfd3/output.txt Total script time: 6.62 mins
|
/botio unittest |
From: Bot.io (Linux)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/eccb34d8fc33a46/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/8a50ce356b06d6c/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/eccb34d8fc33a46/output.txt Total script time: 5.63 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/8a50ce356b06d6c/output.txt Total script time: 6.47 mins
|
/botio unittest |
From: Bot.io (Linux)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/2e3bc45239cba04/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/9e10abe19ae611d/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/2e3bc45239cba04/output.txt Total script time: 5.63 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/9e10abe19ae611d/output.txt Total script time: 6.64 mins
|
/botio unittest |
From: Bot.io (Linux)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/e52b48591f602ab/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/80f578b50f94a53/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/e52b48591f602ab/output.txt Total script time: 5.23 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/80f578b50f94a53/output.txt Total script time: 6.60 mins
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice diff stats (+146 −528); thank you for the patch!
Convert the files in the `/test/unit` folder to ES6 modules
The SystemJS module loader has a very low impact on the runtime of the unit tests. Locally, the runtime increased from 20.4 seconds to 21.9 seconds, so that should be fine.
Slightly easier reviewing with https://github.com/mozilla/pdf.js/pull/8298/files?w=1.