Skip to content
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

Merged
merged 1 commit into from
Dec 10, 2020

Conversation

calixteman
Copy link
Contributor

  • run with gulp integrationtest

Copy link
Contributor

@timvandermeij timvandermeij left a 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.

test/unit/integration.js Outdated Show resolved Hide resolved
gulpfile.js Outdated Show resolved Hide resolved
test/test.js Outdated Show resolved Hide resolved
test/test.js Outdated Show resolved Hide resolved
test/test.js Outdated Show resolved Hide resolved
const page = pages[0];
await page.goto(startUrl, { timeout: 0 });
};
if (browserName === "chrome") {
Copy link
Contributor

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?

Copy link
Contributor Author

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.

test/unit/integration.js Outdated Show resolved Hide resolved
test/unit/integration.js Outdated Show resolved Hide resolved
test/unit/integration.js Outdated Show resolved Hide resolved
test/unit/integration.js Outdated Show resolved Hide resolved
@calixteman calixteman force-pushed the interaction branch 2 times, most recently from a148246 to 0e59e99 Compare December 1, 2020 15:36
Copy link
Contributor

@brendandahl brendandahl left a 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

if (typeof PDFJSDev === "undefined" || PDFJSDev.test("MOZCENTRAL")) {


const describes = [];

global.describe = (desc, filename, selector, callback) => {
Copy link
Contributor

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.

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Dec 3, 2020

For getting various events when a page is rendered and forms are ready we can enable the dom dispatch.

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 TESTING-builds.

Edit: The supported way of accessing events globally, in a GENERIC-build of the default viewer (which is what we should be testing here), would be along the lines of https://github.com/mozilla/pdf.js/wiki/Third-party-viewer-usage, i.e.

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);
    });
  });
});

It's currently only enabled for development and mozcentral builds, so we'd need to add a new build flag or option to

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

@timvandermeij
Copy link
Contributor

Related: mozilla/botio-files-pdfjs#28

@calixteman calixteman force-pushed the interaction branch 3 times, most recently from 18f6a34 to c1f96de Compare December 9, 2020 18:49
@brendandahl
Copy link
Contributor

/botio-linux integrationtest

@pdfjsbot
Copy link

pdfjsbot commented Dec 9, 2020

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @brendandahl received. Current queue size: 0

Live output at: http://54.67.70.0:8877/eb1d4532db58d02/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Dec 9, 2020

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/eb1d4532db58d02/output.txt

Total script time: 2.52 mins

  • Integration Tests: Passed

@brendandahl
Copy link
Contributor

/botio-windows integrationtest

@pdfjsbot
Copy link

pdfjsbot commented Dec 9, 2020

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @brendandahl received. Current queue size: 0

Live output at: http://3.101.106.178:8877/b4b089de48464a6/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Dec 9, 2020

From: Bot.io (Windows)


Failed

Full output at http://3.101.106.178:8877/b4b089de48464a6/output.txt

Total script time: 3.36 mins

  • Integration Tests: FAILED

@brendandahl
Copy link
Contributor

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.

@calixteman calixteman force-pushed the interaction branch 2 times, most recently from fcc1697 to d3cb3ab Compare December 10, 2020 12:09
@calixteman
Copy link
Contributor Author

/botio-linux integrationtest

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.67.70.0:8877/ee4b383af23bc29/output.txt

gulpfile.js Outdated Show resolved Hide resolved
gulpfile.js Outdated Show resolved Hide resolved
test/integration/annotation_spec.js Outdated Show resolved Hide resolved
@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/ee4b383af23bc29/output.txt

Total script time: 2.40 mins

  • Integration Tests: Passed

Comment on lines 263 to 264
typeof PDFJSDev === "undefined" ||
PDFJSDev.test("!PRODUCTION && !INTEGRATIONTEST")
Copy link
Collaborator

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?

Copy link
Contributor Author

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

Copy link
Collaborator

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 of PDFJSDev.test("!PRODUCTION") ?

Those two formats are equivalent.

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://3.101.106.178:8877/faba073cd9610f0/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://3.101.106.178:8877/faba073cd9610f0/output.txt

Total script time: 3.38 mins

  • Integration Tests: Passed

@brendandahl
Copy link
Contributor

/botio test

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @brendandahl received. Current queue size: 0

Live output at: http://3.101.106.178:8877/c9a964717dc16bb/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_test from @brendandahl received. Current queue size: 0

Live output at: http://54.67.70.0:8877/bd7a2596622c823/output.txt

Copy link
Contributor

@brendandahl brendandahl left a 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.

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/bd7a2596622c823/output.txt

Total script time: 26.10 mins

  • Font tests: Passed
  • Unit tests: FAILED
  • Integration Tests: FAILED
  • Regression tests: FAILED

Image differences available at: http://54.67.70.0:8877/bd7a2596622c823/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://3.101.106.178:8877/c9a964717dc16bb/output.txt

Total script time: 30.55 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED

Image differences available at: http://3.101.106.178:8877/c9a964717dc16bb/reftest-analyzer.html#web=eq.log

@calixteman
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_test from @calixteman received. Current queue size: 1

Live output at: http://54.67.70.0:8877/7154439e10f2187/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @calixteman received. Current queue size: 1

Live output at: http://3.101.106.178:8877/95dd2507981f2ec/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/7154439e10f2187/output.txt

Total script time: 26.05 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED

Image differences available at: http://54.67.70.0:8877/7154439e10f2187/reftest-analyzer.html#web=eq.log

@calixteman
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.67.70.0:8877/838f315cdd2283c/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @calixteman received. Current queue size: 1

Live output at: http://3.101.106.178:8877/0c190421f889fcc/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://3.101.106.178:8877/95dd2507981f2ec/output.txt

Total script time: 30.33 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED

Image differences available at: http://3.101.106.178:8877/95dd2507981f2ec/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/838f315cdd2283c/output.txt

Total script time: 26.13 mins

  • Font tests: Passed
  • Unit tests: FAILED
  • Integration Tests: Passed
  • Regression tests: FAILED

Image differences available at: http://54.67.70.0:8877/838f315cdd2283c/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://3.101.106.178:8877/0c190421f889fcc/output.txt

Total script time: 32.25 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED

Image differences available at: http://3.101.106.178:8877/0c190421f889fcc/reftest-analyzer.html#web=eq.log

@brendandahl
Copy link
Contributor

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_test from @brendandahl received. Current queue size: 0

Live output at: http://54.67.70.0:8877/90b20ee0335da91/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @brendandahl received. Current queue size: 0

Live output at: http://3.101.106.178:8877/83d68d3ce4c659e/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/90b20ee0335da91/output.txt

Total script time: 26.17 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED

Image differences available at: http://54.67.70.0:8877/90b20ee0335da91/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://3.101.106.178:8877/83d68d3ce4c659e/output.txt

Total script time: 29.92 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED

Image differences available at: http://3.101.106.178:8877/83d68d3ce4c659e/reftest-analyzer.html#web=eq.log

@brendandahl brendandahl merged commit 31ea30a into mozilla:master Dec 10, 2020
@timvandermeij
Copy link
Contributor

Thank you for setting this up! This will be very useful so we can actually start writing integration tests for the viewer now.

@Snuffleupagus Snuffleupagus mentioned this pull request May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants