Rework integrations tests to compare pdf screenshots instead of raw c…

…ontent (#1162)

* Rework integrations tests to compare pdf screenshots instead of raw content

* Update documentation and configuration related to integration/visual tests

.gitignore

@@ -11,5 +11,3 @@ demo/bundle.js
 !demo/browser.html
 .vscode
 coverage
-tests/integration/__pdfs__
-tests/integration/pdfmake/__pdfs__

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ### Unreleased
 
+- Replace integration tests by visual regression tests
 - Fix access permissions in PDF version 1.7ext3
 - Fix Buffer() is deprecation warning
 

CONTRIBUTING.md

@@ -60,24 +60,27 @@ To install the project you need to have `node`
 
 ## Running and writing tests
 
-Tests are run using [Jest](http://jestjs.io/) and are categorized as integration and unit tests. 
+Tests are run using [Jest](http://jestjs.io/) and are categorized as unit and visual tests. 
 
-Integration tests check the pdf output against a reference stored as snapshots. While is served well to avoid regressions it has some disadvantages like small (correct) changes requiring to update all snapshots
+Visual tests check the pdf image screenshot against a reference stored as snapshots.
 
-Unit tests checks behavior os specific classes / methods isolatedly. It covers relatively small portion of code but is preferred way of writing new tests going forward
+Unit tests checks behavior os specific classes / methods isolatedly.
 
 Tests commands
 * `npm run test`: Run all tests
 * `npm run test:unit`: Run unit tests
-* `npm run test:integration`: Run integration tests
+* `npm run test:visual`: Run visual tests
 
-To write new tests, look for the *.spec.js files at `test/unit` and `test/integration` as examples
+To write new tests, look for the *.spec.js files at `test/unit` and `test/visual` as examples
 
+> Visual tests should use an embedded font, instead of system fonts, to ensure uniform rendering between different environments
 
 ## Submitting a Pull Request
 
 Please go through existing issues and pull requests to check if somebody else is already working on it.
 
 Also, make sure to run the tests and lint the code before you commit your changes.
 
-**Preferentially, tests should be added to check the changed behavior even if is a bug fix. Unit tests are preferred over integration ones**
+> Tests should be added to check the changed behavior even if is a bug fix.
+If the proposed change affects document structure a unit test should be added, if affects rendering, add a visual test
+

package.json

@@ -23,19 +23,22 @@
   },
   "bugs": "https://github.com/foliojs/pdfkit/issues",
   "devDependencies": {
-    "@babel/core": "^7.7.2",
-    "@babel/plugin-external-helpers": "^7.0.0",
-    "@babel/preset-env": "^7.7.1",
-    "babel-jest": "^24.9.0",
+    "@babel/core": "^7.11.6",
+    "@babel/plugin-external-helpers": "^7.10.4",
+    "@babel/preset-env": "^7.11.5",
+    "babel-jest": "^26.3.0",
     "blob-stream": "^0.1.2",
     "brace": "^0.11.1",
     "brfs": "~2.0.2",
     "browserify": "^16.5.0",
+    "canvas": "^2.6.1",
     "codemirror": "~5.49.2",
     "eslint": "^6.6.0",
     "iconv-lite": "^0.5.0",
-    "jest": "^24.9.0",
+    "jest": "^26.4.2",
+    "jest-screenshot": "^0.3.1",
     "markdown": "~0.5.0",
+    "pdfjs-dist": "^2.4.456",
     "prettier": "1.19.1",
     "pug": "^2.0.4",
     "rollup": "^1.27.0",
@@ -58,7 +61,7 @@
     "docs": "npm run pdf-guide && npm run website && npm run browser-demo",
     "prettier": "prettier {lib,tests,demo,docs}/**/*.js",
     "test": "jest -i",
-    "test:integration": "jest integration/ -i",
+    "test:visual": "jest visual/ -i",
     "test:unit": "jest unit/"
   },
   "main": "js/pdfkit.js",
@@ -80,6 +83,10 @@
     "testURL": "http://localhost/",
     "setupFilesAfterEnv": [
       "<rootDir>/tests/unit/setupTests.js"
+    ],
+    "reporters": [
+      "default",
+      "jest-screenshot/reporter"
     ]
   }
-}
+}

tests/unit/annotations.spec.js

@@ -54,7 +54,10 @@ describe('Annotations', () => {
 
       const docData = logData(document);
 
-      document.text('Go to url', { link: 'http://www.example.com', continued: true });
+      document.text('Go to url', {
+        link: 'http://www.example.com',
+        continued: true
+      });
       document.text('continued link');
 
       expect(docData).toContainChunk([
@@ -79,7 +82,10 @@ describe('Annotations', () => {
 
       const docData = logData(document);
 
-      document.text('Go to url', { link: 'http://www.example.com', continued: true });
+      document.text('Go to url', {
+        link: 'http://www.example.com',
+        continued: true
+      });
       document.text('no continued link', { link: null });
 
       // console.log(docData);
@@ -91,10 +97,7 @@ describe('Annotations', () => {
 >>`
       ]);
 
-      expect(docData).not.toContainChunk([
-        `14 0 obj`
-      ]);
+      expect(docData).not.toContainChunk([`14 0 obj`]);
     });
-
   });
 });

tests/unit/document.spec.js

@@ -34,17 +34,16 @@ describe('PDFDocument', () => {
   });
 
   describe('document info', () => {
-
     test('accepts properties with value undefined', () => {
-      expect(() => new PDFDocument({ info: { Title: undefined }}))
-        .not.toThrow(new TypeError("Cannot read property 'toString' of undefined"));
+      expect(() => new PDFDocument({ info: { Title: undefined } })).not.toThrow(
+        new TypeError("Cannot read property 'toString' of undefined")
+      );
     });
-    
+
     test('accepts properties with value null', () => {
-      expect(() => new PDFDocument({ info: { Title: null }}))
-        .not.toThrow(new TypeError("Cannot read property 'toString' of null"));
+      expect(() => new PDFDocument({ info: { Title: null } })).not.toThrow(
+        new TypeError("Cannot read property 'toString' of null")
+      );
     });
-
   });
-
 });

tests/unit/setupTests.js

@@ -1,4 +1,5 @@
 import matcher from './toContainChunk';
-
+import { setupJestScreenshot } from 'jest-screenshot';
 
 expect.extend(matcher);
+setupJestScreenshot();

tests/unit/text.spec.js

@@ -171,16 +171,16 @@ Q
       // we look out for a side effect of this infinite loop, witch is adding and infinite number of pages.
       // Nomaly, there should not be any page added to the document.
 
-      document.on("pageAdded", () => {
-		const pageRange = document.bufferedPageRange();
-		const newPageIndex = pageRange.start + pageRange.count;
+      document.on('pageAdded', () => {
+        const pageRange = document.bufferedPageRange();
+        const newPageIndex = pageRange.start + pageRange.count;
         // We try restrict the fail condition to only infinite loop, so we wait for several pages to be added.
         if (newPageIndex > 10) {
-          throw new Error("Infinite loop detected");
+          throw new Error('Infinite loop detected');
         }
       });
 
-      document.text(text, 10, 10, {width: 2});
+      document.text(text, 10, 10, { width: 2 });
       document.end();
 
       expect(docData).toContainChunk([

tests/unit/toContainChunk/index.js

@@ -30,7 +30,7 @@ const failMessage = (utils, data, chunk, headIndex) => () => {
 };
 
 export default {
-  toContainChunk (data, chunk) {
+  toContainChunk(data, chunk) {
     const headIndex = data.indexOf(chunk[0]);
     let pass = headIndex !== -1;
     if (pass) {