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

Tests for Viewer #6505

Closed
brendandahl opened this issue Oct 5, 2015 · 9 comments
Closed

Tests for Viewer #6505

brendandahl opened this issue Oct 5, 2015 · 9 comments
Labels

Comments

@brendandahl
Copy link
Contributor

There is currently too much manual testing required to land changes to the viewer. There are some tests that currently live in mozilla central for the viewer, but I'd like to see more tests live in the pdf.js repo. I don't have much of an opinion on what framework we use, but it would nice if this ran on travis.

-- Update --
Overall, I think it would be best to follow a test pyramid. How we can effectively unit test the viewer is a harder problem. Some colleagues working on firefox devtools and hello have had a lot positive things to say about testing their react pieces, but rewriting with react would be no minor under taking.

@timvandermeij
Copy link
Contributor

Do you already have ideas on how this should work and do you know of any frameworks that can be used? I recall having looked into this a long time ago and back then there did not appear to be much for our use case. I agree that it would be nice if it could run on Travis and recently I read that Travis can also start browsers like Firefox and Chrome, only not IE I guess ;-)

How do the tests at Mozilla Central work? Do they check for example visibility of DOM elements or do they use screenshots? The latter is more difficult on Travis, the former should be doable.

@brendandahl
Copy link
Contributor Author

My initial thoughts were selenium or marionette( though I don't think this supports chrome). We could also use our current framework and try to build on top of that. Or there is also casperjs, but this runs older version of gecko and webkit.

The mozilla central tests usually just check the existence of something, you can fine them at:
https://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/test/

@timvandermeij
Copy link
Contributor

Marking as a good beginner bug as this does not require much knowledge of the PDF.js codebase to work on. Perhaps contributors from other JS projects have experience with integration testing to help us out with this. The first step will be to choose and add the integration testing framework. After that, we should iterate on adding tests.

@brendandahl brendandahl changed the title Integration Tests for Viewer Tests for Viewer Mar 30, 2016
@Skaty
Copy link
Contributor

Skaty commented Jan 9, 2017

@timvandermeij May I know if this issue is still outstanding?

@timvandermeij
Copy link
Contributor

Yes, this issue is still open. Feel free to discuss this with us on IRC if you have questions or want to know how to proceed with this.

@is2ei
Copy link

is2ei commented Jun 3, 2018

@timvandermeij
Hi, I'm Issei.
Could I work on this issue?

@timvandermeij
Copy link
Contributor

Yes, feel free to work on this.

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented May 13, 2021

Isn't this issue essentially fixed by PR #12668 now, since the integrationtest framework can (and already do) cover various aspects of the default viewer.

@timvandermeij
Copy link
Contributor

Yes, indeed. More tests can be added when we need them, but the framework is now in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants