fix: correctly respect logging option (niklasvh#2013)

* test: update to using jest for unit tests * remove mocha types * revert to using mocha for testrunner.ts * add logger unit testing * fix reftest previewer scaling * fix LoggerOptions to interface * fix linting
pawelzwronek · Oct 13, 2019 · 1a81bc9 · 1a81bc9
1 parent 7c638aa
commit 1a81bc9
Show file tree

Hide file tree

Showing 17 changed files with 2,624 additions and 236 deletions.
diff --git a/jest.config.js b/jest.config.js
@@ -0,0 +1,5 @@
+module.exports = {
+    preset: 'ts-jest',
+    testEnvironment: 'jsdom',
+    roots: ['src']
+};
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -29,6 +29,7 @@
     "@babel/preset-flow": "^7.0.0",
     "@types/chai": "^4.1.7",
     "@types/glob": "^7.1.1",
+    "@types/jest": "^24.0.18",
     "@types/mocha": "^5.2.6",
     "@types/node": "^11.13.2",
     "@types/platform": "^1.3.2",
@@ -53,6 +54,7 @@
     "filenamify-url": "1.0.0",
     "glob": "7.1.3",
     "html2canvas-proxy": "1.0.1",
+    "jest": "^24.9.0",
     "jquery": "^3.4.0",
     "js-polyfills": "^0.1.42",
     "karma": "^4.0.1",
@@ -79,6 +81,7 @@
     "serve-index": "^1.9.1",
     "slash": "1.0.0",
     "standard-version": "^5.0.2",
+    "ts-jest": "^24.1.0",
     "ts-loader": "^5.3.3",
     "ts-node": "^8.0.3",
     "typescript": "^3.4.3",
@@ -99,7 +102,7 @@
     "format": "prettier --write \"{src,www/src,tests,scripts}/**/*.ts\"",
     "lint": "eslint src/**/*.ts",
     "test": "npm run lint && npm run unittest && npm run karma",
-    "unittest": "mocha --require ts-node/register src/**/__tests__/*.ts",
+    "unittest": "jest",
     "karma": "node karma",
     "watch": "rollup -c rollup.config.ts -w",
     "watch:unittest": "mocha --require ts-node/register --watch-extensions ts -w src/**/__tests__/*.ts",

diff --git a/src/core/__mocks__/cache-storage.ts b/src/core/__mocks__/cache-storage.ts
@@ -0,0 +1,22 @@
+class MockCache {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    private readonly _cache: {[key: string]: Promise<any>};
+
+    constructor() {
+        this._cache = {};
+    }
+
+    addImage(src: string): Promise<void> {
+        const result = Promise.resolve();
+        this._cache[src] = result;
+        return result;
+    }
+}
+
+const current = new MockCache();
+
+export class CacheStorage {
+    static getInstance(): MockCache {
+        return current;
+    }
+}
diff --git a/src/core/__mocks__/features.ts b/src/core/__mocks__/features.ts
@@ -0,0 +1,8 @@
+export const FEATURES = {
+    SUPPORT_RANGE_BOUNDS: true,
+    SUPPORT_SVG_DRAWING: true,
+    SUPPORT_FOREIGNOBJECT_DRAWING: true,
+    SUPPORT_CORS_IMAGES: true,
+    SUPPORT_RESPONSE_TYPE: true,
+    SUPPORT_CORS_XHR: true
+};
diff --git a/src/core/__tests__/cache-storage.ts b/src/core/__tests__/cache-storage.ts
@@ -1,6 +1,49 @@
 import {deepStrictEqual, fail} from 'assert';
 import {FEATURES} from '../features';
-import {createMockContext, proxy} from './mock-context';
+import {CacheStorage} from '../cache-storage';
+import {Logger} from '../logger';
+
+const proxy = 'http://example.com/proxy';
+
+const createMockContext = (origin: string, opts = {}) => {
+    const context = {
+        location: {
+            href: origin
+        },
+        document: {
+            createElement(_name: string) {
+                let _href = '';
+                return {
+                    set href(value: string) {
+                        _href = value;
+                    },
+                    get href() {
+                        return _href;
+                    },
+                    get protocol() {
+                        return new URL(_href).protocol;
+                    },
+                    get hostname() {
+                        return new URL(_href).hostname;
+                    },
+                    get port() {
+                        return new URL(_href).port;
+                    }
+                };
+            }
+        }
+    };
+
+    CacheStorage.setContext(context as Window);
+    Logger.create({id: 'test', enabled: false});
+    return CacheStorage.create('test', {
+        imageTimeout: 0,
+        useCORS: false,
+        allowTaint: false,
+        proxy,
+        ...opts
+    });
+};
 
 const images: ImageMock[] = [];
 const xhr: XMLHttpRequestMock[] = [];

diff --git a/src/core/__tests__/logger.ts b/src/core/__tests__/logger.ts
@@ -0,0 +1,27 @@
+import {Logger} from '../logger';
+
+describe('logger', () => {
+    let infoSpy: any;
+
+    beforeEach(() => {
+        infoSpy = jest.spyOn(console, 'info').mockImplementation(() => {});
+    });
+
+    afterEach(() => {
+        infoSpy.mockRestore();
+    });
+
+    it('should call console.info when logger enabled', () => {
+        const id = Math.random().toString();
+        const logger = new Logger({id, enabled: true});
+        logger.info('testing');
+        expect(infoSpy).toHaveBeenLastCalledWith(id, expect.stringMatching(/\d+ms/), 'testing');
+    });
+
+    it("shouldn't call console.info when logger disabled", () => {
+        const id = Math.random().toString();
+        const logger = new Logger({id, enabled: false});
+        logger.info('testing');
+        expect(infoSpy).not.toHaveBeenCalled();
+    });
+});
diff --git a/src/core/__tests__/mock-context.ts b/src/core/__tests__/mock-context.ts
diff --git a/src/core/logger.ts b/src/core/logger.ts
@@ -1,31 +1,40 @@
+export interface LoggerOptions {
+    id: string;
+    enabled: boolean;
+}
+
 export class Logger {
     static instances: {[key: string]: Logger} = {};
 
     private readonly id: string;
+    private readonly enabled: boolean;
     private readonly start: number;
 
-    constructor(id: string) {
+    constructor({id, enabled}: LoggerOptions) {
         this.id = id;
+        this.enabled = enabled;
         this.start = Date.now();
     }
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     debug(...args: any) {
-        // eslint-disable-next-line no-console
-        if (typeof window !== 'undefined' && window.console && typeof console.debug === 'function') {
+        if (this.enabled) {
             // eslint-disable-next-line no-console
-            console.debug(this.id, `${this.getTime()}ms`, ...args);
-        } else {
-            this.info(...args);
+            if (typeof window !== 'undefined' && window.console && typeof console.debug === 'function') {
+                // eslint-disable-next-line no-console
+                console.debug(this.id, `${this.getTime()}ms`, ...args);
+            } else {
+                this.info(...args);
+            }
         }
     }
 
     getTime(): number {
         return Date.now() - this.start;
     }
 
-    static create(id: string) {
-        Logger.instances[id] = new Logger(id);
+    static create(options: LoggerOptions) {
+        Logger.instances[options.id] = new Logger(options);
     }
 
     static destroy(id: string) {
@@ -42,21 +51,25 @@ export class Logger {
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     info(...args: any) {
-        // eslint-disable-next-line no-console
-        if (typeof window !== 'undefined' && window.console && typeof console.info === 'function') {
+        if (this.enabled) {
             // eslint-disable-next-line no-console
-            console.info(this.id, `${this.getTime()}ms`, ...args);
+            if (typeof window !== 'undefined' && window.console && typeof console.info === 'function') {
+                // eslint-disable-next-line no-console
+                console.info(this.id, `${this.getTime()}ms`, ...args);
+            }
         }
     }
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     error(...args: any) {
-        // eslint-disable-next-line no-console
-        if (typeof window !== 'undefined' && window.console && typeof console.error === 'function') {
+        if (this.enabled) {
             // eslint-disable-next-line no-console
-            console.error(this.id, `${this.getTime()}ms`, ...args);
-        } else {
-            this.info(...args);
+            if (typeof window !== 'undefined' && window.console && typeof console.error === 'function') {
+                // eslint-disable-next-line no-console
+                console.error(this.id, `${this.getTime()}ms`, ...args);
+            } else {
+                this.info(...args);
+            }
         }
     }
 }
diff --git a/src/css/property-descriptors/__tests__/background-tests.ts b/src/css/property-descriptors/__tests__/background-tests.ts
@@ -4,15 +4,13 @@ import {backgroundImage} from '../background-image';
 import {CSSImageType} from '../../types/image';
 import {pack} from '../../types/color';
 import {deg} from '../../types/angle';
-import {createMockContext} from '../../../core/__tests__/mock-context';
-import {CacheStorage} from '../../../core/cache-storage';
+
+jest.mock('../../../core/cache-storage');
+jest.mock('../../../core/features');
 
 const backgroundImageParse = (value: string) => backgroundImage.parse(Parser.parseValues(value));
 
 describe('property-descriptors', () => {
-    before(() => {
-        CacheStorage.attachInstance(createMockContext('http://example.com'));
-    });
     describe('background-image', () => {
         it('none', () => deepStrictEqual(backgroundImageParse('none'), []));
 

diff --git a/src/css/types/__tests__/image-tests.ts b/src/css/types/__tests__/image-tests.ts
@@ -8,6 +8,9 @@ import {deg} from '../angle';
 const parse = (value: string) => image.parse(Parser.parseValue(value));
 const colorParse = (value: string) => color.parse(Parser.parseValue(value));
 
+jest.mock('../../../core/cache-storage');
+jest.mock('../../../core/features');
+
 describe('types', () => {
     describe('<image>', () => {
         describe('parsing', () => {

diff --git a/src/index.ts b/src/index.ts
@@ -11,7 +11,7 @@ import {ForeignObjectRenderer} from './render/canvas/foreignobject-renderer';
 export type Options = CloneOptions &
     RenderOptions &
     ResourceOptions & {
-        backgroundColor: string;
+        backgroundColor: string | null;
         foreignObjectRendering: boolean;
         logging: boolean;
         removeContainer?: boolean;
@@ -76,7 +76,7 @@ const renderElement = async (element: HTMLElement, opts: Partial<Options>): Prom
 
     const windowBounds = new Bounds(options.scrollX, options.scrollY, options.windowWidth, options.windowHeight);
 
-    Logger.create(instanceName);
+    Logger.create({id: instanceName, enabled: options.logging});
     Logger.getInstance(instanceName).debug(`Starting document clone`);
     const documentCloner = new DocumentCloner(element, {
         id: instanceName,
@@ -101,7 +101,8 @@ const renderElement = async (element: HTMLElement, opts: Partial<Options>): Prom
         : COLORS.TRANSPARENT;
 
     const bgColor = opts.backgroundColor;
-    const defaultBackgroundColor = typeof bgColor === 'string' ? parseColor(bgColor) : bgColor === null ? COLORS.TRANSPARENT : 0xffffffff;
+    const defaultBackgroundColor =
+        typeof bgColor === 'string' ? parseColor(bgColor) : bgColor === null ? COLORS.TRANSPARENT : 0xffffffff;
 
     const backgroundColor =
         element === ownerDocument.documentElement

diff --git a/src/render/canvas/canvas-renderer.ts b/src/render/canvas/canvas-renderer.ts
@@ -578,10 +578,10 @@ export class CanvasRenderer {
 
                 ctx.fillStyle = gradient;
                 ctx.fillRect(0, 0, width, height);
-                if ((width > 0) && (height > 0)) {
-					const pattern = this.ctx.createPattern(canvas, 'repeat') as CanvasPattern;
-					this.renderRepeat(path, pattern, x, y);
-				}
+                if (width > 0 && height > 0) {
+                    const pattern = this.ctx.createPattern(canvas, 'repeat') as CanvasPattern;
+                    this.renderRepeat(path, pattern, x, y);
+                }
             } else if (isRadialGradient(backgroundImage)) {
                 const [path, left, top, width, height] = calculateBackgroundRendering(container, index, [
                     null,

diff --git a/tests/rollup.config.ts b/tests/rollup.config.ts
@@ -36,7 +36,7 @@ export default {
         // Allow json resolution
         json(),
         // Compile TypeScript files
-        typescript({useTsconfigDeclarationDir: true}),
+        typescript({useTsconfigDeclarationDir: true, tsconfig: resolve(__dirname, 'tsconfig.json')}),
         // Allow bundling cjs modules (unlike webpack, rollup doesn't understand cjs)
         commonjs({
             include: 'node_modules/**',

diff --git a/tests/tsconfig.json b/tests/tsconfig.json
@@ -0,0 +1,6 @@
+{
+    "extends": "../tsconfig.json",
+    "compilerOptions": {
+        "types": ["node", "mocha"]
+    }
+}
diff --git a/tsconfig.json b/tsconfig.json
@@ -7,7 +7,7 @@
         "strictNullChecks": true,
         "strictPropertyInitialization": true,
         "resolveJsonModule": true,
-        "typeRoots": [ "./types", "./node_modules/@types"],
+        "types": ["node", "jest"],
         "target": "es5",
         "lib": ["es2015", "dom"],
         "sourceMap": true,

diff --git a/www/src/preview.ts b/www/src/preview.ts
@@ -42,6 +42,8 @@ function onBrowserChange(browserTest: Test) {
         previewImage.src = `/results/${browserTest.screenshot}.png`;
         if (browserTest.devicePixelRatio > 1) {
             previewImage.style.transform = `scale(${1 / browserTest.devicePixelRatio})`;
+        } else {
+            previewImage.style.transform = '';
         }
     }
 }