-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Add some integration tests using puppeteer #12668
Conversation
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 idea! I primarily have some comments that should make this more generally usable. We should think about if we want to make this part of the bot/Travis tests already or to do that later.
const page = pages[0]; | ||
await page.goto(startUrl, { timeout: 0 }); | ||
}; | ||
if (browserName === "chrome") { |
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.
Could you add a comment why these additional Chrome and Firefox preferences are necessary?
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.
About option for Chrome, they're required at least on my machine because without them chrome is crashing!
And as a side effect I just discovered that unit tests are running on chrome too.
a148246
to
0e59e99
Compare
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.
For getting various events when a page is rendered and forms are ready we can enable the dom dispatch. It's currently only enabled for development and mozcentral builds, so we'd need to add a new build flag or option to
Line 806 in a618b02
if (typeof PDFJSDev === "undefined" || PDFJSDev.test("MOZCENTRAL")) { |
test/integration/integration-boot.js
Outdated
|
||
const describes = []; | ||
|
||
global.describe = (desc, filename, selector, callback) => { |
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.
Instead of adding a new test framework, can we load jasmine (which we already use)? I was hoping to use the integration tests for more than just form tests which this is pretty tailored too. This current method would then be a helper that a test would use.
While we used to dispatch events to the DOM, that was purposely removed since it's not a great API in the general case (and only existed for backwards compatibility); hence can we please not re-add that sort of functionality here! If it's turns out to be absolutely necessary, with no way around it, please at least limit this to only being enabled (using pre-processor statements) for Edit: The supported way of accessing events globally, in a document.addEventListener("webviewerloaded", function() {
PDFViewerApplication.initializedPromise.then(function() {
// The viewer has now been initialized, we can access e.g. the `EventBus`-instance.
PDFViewerApplication.eventBus.on("pagerendered", evt => {
console.log(`Rendered page: ${evt.pageNumber);
});
});
});
The only reason that we even kept this functionality at all, limited to mozilla-central (and nowhere else), was to not break the existing tests in https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/test |
110ae24
to
d3595f6
Compare
d3595f6
to
2b2776a
Compare
Related: mozilla/botio-files-pdfjs#28 |
18f6a34
to
c1f96de
Compare
/botio-linux integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @brendandahl received. Current queue size: 0 Live output at: http://54.67.70.0:8877/eb1d4532db58d02/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/eb1d4532db58d02/output.txt Total script time: 2.52 mins
|
/botio-windows integrationtest |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @brendandahl received. Current queue size: 0 Live output at: http://3.101.106.178:8877/b4b089de48464a6/output.txt |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/b4b089de48464a6/output.txt Total script time: 3.36 mins
|
For the windows failure we need to increase the timeout for the beforeAll (second argument). There's suppose to be a way to change it for everything with jasmine.DEFAULT_TIMEOUT_INTERVAL but I can't seem to get that to apply. |
fcc1697
to
d3cb3ab
Compare
/botio-linux integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.67.70.0:8877/ee4b383af23bc29/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/ee4b383af23bc29/output.txt Total script time: 2.40 mins
|
web/app_options.js
Outdated
typeof PDFJSDev === "undefined" || | ||
PDFJSDev.test("!PRODUCTION && !INTEGRATIONTEST") |
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.
Also, are these changes actually necessary since there should be no build-target setting PRODUCTION = false
anyway?
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.
Good question, there was a !PDFJSDev.test("PRODUCTION")
, is this value different of PDFJSDev.test("!PRODUCTION")
?
And there's a similar use here:
/~https://github.com/mozilla/pdf.js/blob/master/web/app_options.js#L248
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.
there was a
!PDFJSDev.test("PRODUCTION")
, is this value different ofPDFJSDev.test("!PRODUCTION")
?
Those two formats are equivalent.
d3cb3ab
to
f614e0f
Compare
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://3.101.106.178:8877/faba073cd9610f0/output.txt |
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/faba073cd9610f0/output.txt Total script time: 3.38 mins
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @brendandahl received. Current queue size: 0 Live output at: http://3.101.106.178:8877/c9a964717dc16bb/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @brendandahl received. Current queue size: 0 Live output at: http://54.67.70.0:8877/bd7a2596622c823/output.txt |
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.
Let's merge this if the full test suite runs fine, so we can unbreak tests for other PR's.
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/bd7a2596622c823/output.txt Total script time: 26.10 mins
Image differences available at: http://54.67.70.0:8877/bd7a2596622c823/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/c9a964717dc16bb/output.txt Total script time: 30.55 mins
Image differences available at: http://3.101.106.178:8877/c9a964717dc16bb/reftest-analyzer.html#web=eq.log |
f614e0f
to
cfe554f
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 1 Live output at: http://54.67.70.0:8877/7154439e10f2187/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 1 Live output at: http://3.101.106.178:8877/95dd2507981f2ec/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/7154439e10f2187/output.txt Total script time: 26.05 mins
Image differences available at: http://54.67.70.0:8877/7154439e10f2187/reftest-analyzer.html#web=eq.log |
cfe554f
to
1e2cc1f
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.67.70.0:8877/838f315cdd2283c/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 1 Live output at: http://3.101.106.178:8877/0c190421f889fcc/output.txt |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/95dd2507981f2ec/output.txt Total script time: 30.33 mins
Image differences available at: http://3.101.106.178:8877/95dd2507981f2ec/reftest-analyzer.html#web=eq.log |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/838f315cdd2283c/output.txt Total script time: 26.13 mins
Image differences available at: http://54.67.70.0:8877/838f315cdd2283c/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/0c190421f889fcc/output.txt Total script time: 32.25 mins
Image differences available at: http://3.101.106.178:8877/0c190421f889fcc/reftest-analyzer.html#web=eq.log |
* run with `gulp integrationtest`
1e2cc1f
to
5b42ac3
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @brendandahl received. Current queue size: 0 Live output at: http://54.67.70.0:8877/90b20ee0335da91/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @brendandahl received. Current queue size: 0 Live output at: http://3.101.106.178:8877/83d68d3ce4c659e/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/90b20ee0335da91/output.txt Total script time: 26.17 mins
Image differences available at: http://54.67.70.0:8877/90b20ee0335da91/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/83d68d3ce4c659e/output.txt Total script time: 29.92 mins
Image differences available at: http://3.101.106.178:8877/83d68d3ce4c659e/reftest-analyzer.html#web=eq.log |
Thank you for setting this up! This will be very useful so we can actually start writing integration tests for the viewer now. |
gulp integrationtest