Reduce the amount of errors logged, primarily in Node.js/Travis, when running the unit-tests #9769
Conversation
|
/botio unittest |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/ec82977d1760d3b/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/f071896e4e78e99/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/ec82977d1760d3b/output.txt Total script time: 3.67 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/f071896e4e78e99/output.txt Total script time: 6.40 mins
|
| (function checkFontFaceAndImage() { | ||
| // Node.js is missing native support for `@font-face` and `Image`. | ||
| if (isNodeJS()) { | ||
| compatibilityParams.disableFontFace = true; |
There was a problem hiding this comment.
Now that this is set in the library, I think we can simplify:
pdf.js/examples/node/pdf2png/pdf2png.js
Lines 62 to 66 in 57e8676
to:
pdfjsLib.getDocument({ data: rawData, }).then(function (pdfDocument) {It makes the example easier to understand, but note that this is an untested idea, so please check if the compatibility introduced here is indeed included in the library bundle.
There was a problem hiding this comment.
[...] so please check if the compatibility introduced here is indeed included in the library bundle.
Given that https://github.com/mozilla/pdf.js/blob/master/src/display/api_compatibility.js is imported by the API, this should be totally fine; see also
Line 29 in 2921cc0
Furthermore, that fact that the document errors are now gone from the Travis output also shows that including these new Node.js specific compatibility options works fine.
|
Aside from the comment above, this looks good to me. I'll try to complete the review of this and the other PR tomorrow. |
…utils.js`, when running the unit-tests in Node.js/Travis
…ativeImageDecoderSupport` API options in Node.js This should provide a better out-of-the-box experience when using PDF.js in a Node.js environment, since it's missing native support for both `@font-face` and `Image`. Please note that this change *only* affects the default values, hence it's still possible for an API consumer to override those values when calling `getDocument`. Also, prevents "ReferenceError: document is not defined" errors, when running the unit-tests in Node.js/Travis.
…ted, even when errors are thrown in `PDFDocumentLoadingTask.onPassword` callback Please note that while the current code works, both in the viewer and the unit-tests, it can leave the `WorkerTransport._passwordCapability` Promise in a pending state. In the `PasswordRequest` handler, in src/display/api.js, we're returning the Promise from a `capability` object (rather than just a "plain" Promise). While an error thrown anywhere within this handler was fortunately enough to propagate it to the Worker side, it won't cause the Promise (in `WorkerTransport._passwordCapability`) to actually be rejected. Finally note that while we're now catching errors in the `PasswordRequest` handler, those errors are still propagated to the Worker side via the (now) rejected Promise and the existing `return this._passwordCapability.promise;` line. This prevents warnings about uncaught Promises, with messages such as "Error: Worker was destroyed during onPassword callback", when running the unit-tests both in browsers *and* in Node.js/Travis.
… when running the unit-tests in Node.js/Travis Compared to running the unit-tests in "regular" browsers, where any console output won't get mixed up with test output, in Node.js/Travis the test output looks quite noisy. By ignoring `info`/`warn` calls, when running unit-tests in Node.js/Travis, the test output is a lot smaller not to mention that any *actual* failures are more easily spotted.
Snuffleupagus
force-pushed
the
node-unittest-rm-console-errors
branch
from
b75b05b to
11b4613
Compare
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/bce3798ec120aa4/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/f77c2edf77cce51/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/bce3798ec120aa4/output.txt Total script time: 18.85 mins
|
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/f77c2edf77cce51/output.txt Total script time: 24.44 mins
Image differences available at: http://54.215.176.217:8877/f77c2edf77cce51/reftest-analyzer.html#web=eq.log |
|
There's apparently still some warnings being printed in the Node.js/Travis output, coming from unit-testing |
|
Nice work, thank you! |
…onsole-errors Reduce the amount of errors logged, primarily in Node.js/Travis, when running the unit-tests
Split off from PR #9729, as requested in #9729 (comment). /cc @timvandermeij
Compare the output without these patches https://travis-ci.org/mozilla/pdf.js/builds/386523654, to the output with the patches https://travis-ci.org/mozilla/pdf.js/builds/386612114.