-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
Enables some unit tests on travis. #7939
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 to be able to run unit-tests on Travis too!
Looks good to me; with one question below.
If I understand correctly, the only unit-tests that we (currently) cannot run on Travis are ones that load various files themselves (e.g. PDFs, CMAPs, etc.).
However, since it seems like a good idea to run as many of the unit-tests on as possible: Should we also add the metadata_spec.js
, ui_utils_spec.js
and util_spec.js
tests to the test/unit/clitests.json
manifest file?
66ecf60
to
2a85368
Compare
I missed util_spec.js by accident. metadata_spec.js and ui_utils_spec.js (as well as cmap, annotation_layer and api) require DOM objects such as XMLHttpRequest or document. |
{ | ||
"spec_dir": "test/unit", | ||
"spec_files": [ | ||
"cff_parser_spec.js", |
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.
Nit: this line and the ones below are indented with three spaces instead of two.
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.
fixed
2a85368
to
c45300e
Compare
Awesome, thanks! |
Enables some unit tests on travis.
Implements idea from #7179 about reusing the same tests and run portion of them on Travis CI via node.