diff --git a/Contributing.md b/Contributing.md index 72a609f..1efd071 100644 --- a/Contributing.md +++ b/Contributing.md @@ -61,12 +61,18 @@ If the branch contains lot of small commits, you might be asked to squash the co ### 4. Run lint and testing **Run lint** -Make sure that your code follows our [[Style Guide]] and run from the PDF.js folder: +To make sure that your code follows our [[Style Guide]], run the following command from the PDF.js folder: ``` gulp lint ``` +If there are errors, some of them can be fixed automatically by the linting tool. To do so, run the following command: + +``` +gulp lint --fix +``` + _Protip_: If you are a Vim user, then install [Syntastic](https://www.vim.org/scripts/script.php?script_id=2736), install ESLint globally using `npm install -g eslint` and add the following line to your `.vimrc`: ``` @@ -77,30 +83,29 @@ Now you have automatic linting of your changes to JavaScript files whenever you **Run testing** -To ensure your changes did not introduce any regressions you need to run the testing framework. There are four basic types of tests: +To ensure that your changes did not introduce any regressions you need to run the testing framework. We have three different kinds of tests: -* `load` test: checks if the PDF file can be loaded without crashing -* `eq` test: a reference test that takes correctly rendered snapshots and compares them to snapshots from the current code -* `text` test: a reference test that takes snapshots of the `textLayer` overlay and compares them to snapshots from the current code -* `annotations` test: a reference test that takes snapshots of the `annotationLayer` overlay (and the underlying page) and compares them to snapshots from the current code -* `fbf` test: a forward-back-forward test -* Unit tests: Jasmine unit tests that are run separately from the above tests +* Unit tests (ensure that individual units of code work as expected) +* Font tests (special kind of unit tests that use `ttx` for font validation) +* Browser tests (screenshot testing of rendered PDF files in the browser) + +The browser tests are split into the following types of tests: + +* `load` test (checks if the PDF file can be loaded without crashing) +* `eq` test (reference test that takes correctly rendered snapshots and compares them to snapshots from the current code) +* `text` test (reference test that takes snapshots of the text layer overlay and compares them to snapshots from the current code) +* `annotations` test (reference test that takes snapshots of the annotation layer overlay and compares them to snapshots from the current code) +* `fbf` test (forward-back-forward test) **Generating reference images** -The reference tests require you to generate original snapshots for comparison. The snapshots should be generated before you make any changes. If you have already made some changes, `git stash` your work. Then make sure you have created a `browser_manifest.json` file. Copy the example browser manifest located in `test/resources/browser_manifests` to get started: - -``` -cp test/resources/browser_manifests/browser_manifest.json.example test/resources/browser_manifests/browser_manifest.json -``` -Then edit the manifest and make sure it points to the browser(s) you want to use for generating the reference images. Optionally, you can add `"headless": true` to entries to run the tests without a browser window. - -Now we can generate the reference images: +The reference tests require you to generate original snapshots for comparison. The snapshots should be generated before you make any changes. If you have already made some changes, either `git stash` your work or checkout the master branch. Now we can generate the reference images: ``` gulp makeref ``` -You can then run the test suite from the PDF.js root folder after unstashing any changes: + +You can then run the test suite from the PDF.js root folder after unstashing any changes or going back to your feature branch: ``` gulp test @@ -116,24 +121,26 @@ Note that this script only supports adding `eq` tests for all pages of the PDF f **Running unit tests separately** -Unit tests are run when `gulp test` is run, but they can also be run separately two different ways: +Unit tests are run when `gulp test` is run, but they can also be run separately using: -1. In the browser. A web server has to be setup to host the PDF.js files. Tests will be executed by opening the `{url-to-pdf.js}/test/unit/unit_test.html` page. If the web server is started using the `gulp server` command, the URL will be `http://localhost:8888/test/unit/unit_test.html`. -2. Command line. `gulp unittest` will run all the tests using the regression test framework. +``` +gulp unittest +``` ### 5. Push changes to your fork/branch -After lint and all tests pass, push the changes to your fork/branch on GitHub: +After linting and testing passes, push the changes to your branch on GitHub: + ``` git push origin {branch_name} ``` ### 6. Create pull request -Create a pull request on GitHub for your feature branch. The code will then be reviewed and tested further by our contributors and test bot. +Create a pull request on GitHub for your feature branch. The code will then be reviewed and tested further by our contributors and the test bots. -Note that the translations for PDF.js in the `l10n` folder are synchronized with the Aurora branch of Mozilla Firefox. This means that we will only accept pull requests that add strings currently missing in the Aurora branch (as it will take at least six weeks before the most recent translations are in the Aurora branch), but keep in mind that the changes will be overwritten when we synchronize again. +Note that the translations for PDF.js in the `l10n` folder are synchronized with the Nightly branch of Mozilla Firefox. This means that we will only accept pull requests that add strings currently missing in the Nightly branch, but keep in mind that the changes will be overwritten when we synchronize again. ### 7. Code review and automated testing -In addition to the GitHub pull request workflow, it is highly recommended that you communicate with the PDF.js team, for example via the [Matrix room](https://chat.mozilla.org/#/room/#pdfjs:mozilla.org). That will help to find a reviewer for your patch and speed up the review process. The reviewer will kick off further testing and do a code review. +In addition to the GitHub pull request workflow, it is recommended that you communicate with the PDF.js team, for example via the [Matrix room](https://chat.mozilla.org/#/room/#pdfjs:mozilla.org). That will help to find a reviewer for your patch and speed up the review process. The reviewer will kick off further testing and do a code review. You can speed up fetching a remote GitHub branch (possibly belonging to another user) using `git try {username} {branch_name}`. Add the following to the `.git/config` file to be able to do that: ```