From 4db2bd2f817dbbc9c456a657451d25da44ceb647 Mon Sep 17 00:00:00 2001 From: Romain Deltour Date: Thu, 5 Oct 2017 11:05:37 +0200 Subject: [PATCH] fix: various bugs in the report builder - adding data to a report didn't properly override existing data - the report builders are much more permissive in what they accept - add unit tests for the report builder (other builders TBD) - fix "Unexpected Error: this.json.data.images.forEach is not a function" Closes #53 --- src/report/report-builders.js | 28 +++- src/report/report-builders.test.js | 192 ++++++++++++++++++++++ tests/__tests__/parsing.test.js | 5 + tests/data/issue53/EPUB/content_001.xhtml | 10 ++ tests/data/issue53/EPUB/content_002.xhtml | 10 ++ tests/data/issue53/EPUB/image_001.jpg | 1 + tests/data/issue53/EPUB/nav.xhtml | 12 ++ tests/data/issue53/EPUB/package.opf | 27 +++ tests/data/issue53/META-INF/container.xml | 6 + tests/data/issue53/mimetype | 1 + 10 files changed, 283 insertions(+), 9 deletions(-) create mode 100644 src/report/report-builders.test.js create mode 100644 tests/data/issue53/EPUB/content_001.xhtml create mode 100644 tests/data/issue53/EPUB/content_002.xhtml create mode 100644 tests/data/issue53/EPUB/image_001.jpg create mode 100644 tests/data/issue53/EPUB/nav.xhtml create mode 100644 tests/data/issue53/EPUB/package.opf create mode 100644 tests/data/issue53/META-INF/container.xml create mode 100644 tests/data/issue53/mimetype diff --git a/src/report/report-builders.js b/src/report/report-builders.js index 956363c8..57b99449 100644 --- a/src/report/report-builders.js +++ b/src/report/report-builders.js @@ -29,7 +29,7 @@ function calculateResult(assertions) { // add an assertion and recalc the top-level result function withAssertions(obj, assertions) { if (!('assertions' in obj)) obj.assertions = []; - obj.assertions.push(assertions); + obj.assertions = obj.assertions.concat(assertions); obj['earl:result'] = calculateResult(obj.assertions); return obj; } @@ -90,8 +90,8 @@ class ReportBuilder { this._json = { '@type': 'earl:report', '@context': 'http://ace.daisy.org/ns/ace-report._jsonld', - 'dct:title': title, - 'dct:description': description, + 'dct:title': (title == null) ? '' : title.toString(), + 'dct:description': (title == null) ? '' : description.toString(), 'dct:date': new Date().toLocaleString(), 'earl:assertedBy': ACE_DESCRIPTION, outlines: {}, @@ -103,37 +103,47 @@ class ReportBuilder { return this._json; } withA11yMeta(metadata) { + if (!metadata) return this; this._json['a11y-metadata'] = metadata; return this; } withAssertions(assertions) { + if (!assertions) return this; withAssertions(this._json, assertions); return this; } withData(data) { + if (!data) return this; Object.getOwnPropertyNames(data).forEach((key) => { - this._json.data[key] = (Array.isArray(this._json.data[key])) - ? this._json.data[key].push(data[key]) - : data[key]; + if (Array.isArray(this._json.data[key])) { + this._json.data[key] = this._json.data[key].concat(data[key]); + } else { + this._json.data[key] = data[key]; + } }); + return this; } withEPUBOutline(outline) { + if (!outline) return this; this._json.outlines.toc = outline; return this; } withHeadingsOutline(outline) { + if (!outline) return this; this._json.outlines.headings = outline; return this; } withHTMLOutline(outline) { + if (!outline) return this; this._json.outlines.html = outline; return this; } withProperties(properties) { + if (!properties) return this; Object.getOwnPropertyNames(properties).forEach((key) => { - this._json.properties[key] = (key in properties) - ? this._json.properties[key] || properties[key] - : properties[key]; + this._json.properties[key] = (key in this._json.properties) + ? this._json.properties[key] || Boolean(properties[key]) + : Boolean(properties[key]); }); return this; } diff --git a/src/report/report-builders.test.js b/src/report/report-builders.test.js new file mode 100644 index 00000000..66789712 --- /dev/null +++ b/src/report/report-builders.test.js @@ -0,0 +1,192 @@ +'use strict'; + +const builders = require('./report-builders'); +const pkg = require('../../package'); + +describe('report builder', () => { + let report; + + beforeEach(() => { + report = new builders.ReportBuilder(); + }); + + describe('creating a new report', () => { + test('new report is well formed', () => { + expect(report).toBeDefined(); + expect(report.build()).toBeDefined(); + expect(report.build()).toMatchObject({ + '@type': 'earl:report', + '@context': 'http://ace.daisy.org/ns/ace-report.jsonld', + 'dct:title': 'Ace Report', + 'dct:description': 'Report on automated accessibility checks for EPUB', + 'earl:assertedBy': { + '@type': 'earl:software', + 'doap:name': 'DAISY Ace', + 'doap:description': 'DAISY Accessibility Checker for EPUB', + 'doap:homepage': 'http://ace.daisy.org', + 'doap:created': '2017-07-01', + 'doap:release': { 'doap:revision': pkg.version }, + }, + }); + expect(report.build().data).toBeDefined(); + expect(report.build().outlines).toBeDefined(); + expect(report.build().properties).toBeDefined(); + expect(report.build().assertions).toBeUndefined(); + }); + test('new report args are used', () => { + report = new builders.ReportBuilder('Title', 'Desc'); + expect(report).toBeDefined(); + expect(report.build()).toBeDefined(); + expect(report.build()).toMatchObject({ + 'dct:title': 'Title', + 'dct:description': 'Desc', + }); + }); + test('new report undefined args are ignored', () => { + report = new builders.ReportBuilder(undefined, undefined); + expect(report).toBeDefined(); + expect(report.build()).toBeDefined(); + expect(report.build()).toMatchObject({ + 'dct:title': 'Ace Report', + 'dct:description': 'Report on automated accessibility checks for EPUB', + }); + }); + test('new report null args contains empty string values', () => { + report = new builders.ReportBuilder(null, null); + expect(report).toBeDefined(); + expect(report.build()).toBeDefined(); + expect(report.build()).toMatchObject({ + 'dct:title': '', + 'dct:description': '', + }); + }); + test('new report with object args contains toString values', () => { + report = new builders.ReportBuilder({}, {}); + expect(report).toBeDefined(); + expect(report.build()).toBeDefined(); + expect(report.build()).toMatchObject({ + 'dct:title': '[object Object]', + 'dct:description': '[object Object]', + }); + }); + }); + describe('adding assertions', () => { + let assertions; + test('adding null assertion is ignored', () => { + expect(report.withAssertions(null)).toBeDefined(); + expect(report.build().assertions).toBeUndefined(); + }); + test('adding undefined assertion is ignored', () => { + expect(report.withAssertions(undefined)).toBeDefined(); + expect(report.build().assertions).toBeUndefined(); + }); + test('adding single assertion to empty report creates a new array', () => { + expect(report.withAssertions({ foo: 'foo' })).toBeDefined(); + assertions = report.build().assertions; + expect(assertions).toBeDefined(); + expect(Array.isArray(assertions)).toBe(true); + expect(assertions.length).toBe(1); + expect(assertions[0]).toEqual({ foo: 'foo' }); + }); + test('adding single assertion adds to existing assertions', () => { + expect(report.withAssertions({ foo: 'foo' })).toBeDefined(); + assertions = report.build().assertions; + expect(assertions).toBeDefined(); + expect(Array.isArray(assertions)).toBe(true); + expect(assertions.length).toBe(1); + expect(assertions[0]).toEqual({ foo: 'foo' }); + }); + test('adding assertions array to empty report copies it to the report', () => { + expect(report.withAssertions([ + { foo: 'foo' }, { foo: 'foo' }, + ])).toBeDefined(); + assertions = report.build().assertions; + expect(assertions).toBeDefined(); + expect(Array.isArray(assertions)).toBe(true); + expect(assertions.length).toBe(2); + expect(assertions[0]).toEqual({ foo: 'foo' }); + }); + test('adding assertions array concatenates to existing array', () => { + expect(report.withAssertions({ bar: 'bar' })).toBeDefined(); + expect(report.withAssertions([ + { foo: 'foo' }, { foo: 'foo' }, + ])).toBeDefined(); + assertions = report.build().assertions; + expect(assertions).toBeDefined(); + expect(Array.isArray(assertions)).toBe(true); + expect(assertions.length).toBe(3); + expect(assertions[0]).toEqual({ bar: 'bar' }); + expect(assertions[1]).toEqual({ foo: 'foo' }); + }); + }); + describe('adding data', () => { + let data; + beforeEach(() => { + data = report.build().data; + }); + test('adding null data` is ignored', () => { + expect(report.withData(null)).toBeDefined(); + expect(data).toEqual({}); + }); + test('adding undefined data is ignored', () => { + expect(report.withData(undefined)).toBeDefined(); + expect(data).toEqual({}); + }); + test('adding data to new report populates the data section', () => { + expect(report.withData({ foo: 'foo' })).toBeDefined(); + expect(data).toEqual({ foo: 'foo' }); + }); + test('adding data is merged in existing data', () => { + expect(report.withData({ foo: 'foo' })).toBeDefined(); + expect(report.withData({ bar: 'bar' })).toBeDefined(); + expect(data).toEqual({ foo: 'foo', bar: 'bar' }); + }); + test('adding data overrides existing scalar data', () => { + expect(report.withData({ foo: 'foo' })).toBeDefined(); + expect(report.withData({ foo: 'bar' })).toBeDefined(); + expect(data).toEqual({ foo: 'bar' }); + }); + test('adding data adds to existing array data', () => { + expect(report.withData({ foo: ['foo'] })).toBeDefined(); + expect(report.withData({ foo: ['bar'] })).toBeDefined(); + expect(data).toEqual({ foo: ['foo', 'bar'] }); + }); + }); + describe('adding properties', () => { + let properties; + beforeEach(() => { + properties = report.build().properties; + }); + test('adding null properties` is ignored', () => { + expect(report.withProperties(null)).toBeDefined(); + expect(properties).toEqual({}); + }); + test('adding undefined properties is ignored', () => { + expect(report.withProperties(undefined)).toBeDefined(); + expect(properties).toEqual({}); + }); + test('adding properties to new report populates the properties section', () => { + expect(report.withProperties({ foo: true })).toBeDefined(); + expect(properties).toEqual({ foo: true }); + }); + test('adding properties augments existing properties', () => { + expect(report.withProperties({ foo: true })).toBeDefined(); + expect(report.withProperties({ bar: true })).toBeDefined(); + expect(properties).toEqual({ foo: true, bar: true }); + }); + test('adding properties keeps existing true properties', () => { + expect(report.withProperties({ foo: true })).toBeDefined(); + expect(report.withProperties({ foo: false })).toBeDefined(); + expect(properties).toEqual({ foo: true }); + }); + test('adding properties booleanizes values', () => { + expect(report.withProperties({ foo: 'hello', bar: null })).toBeDefined(); + expect(properties).toEqual({ foo: true, bar: false }); + }); + test('adding properties booleanizes values when overrinding too', () => { + expect(report.withProperties({ foo: false })).toBeDefined(); + expect(report.withProperties({ foo: [] })).toBeDefined(); + expect(properties).toEqual({ foo: true }); + }); + }); +}); diff --git a/tests/__tests__/parsing.test.js b/tests/__tests__/parsing.test.js index 04482139..e7b073fe 100644 --- a/tests/__tests__/parsing.test.js +++ b/tests/__tests__/parsing.test.js @@ -40,3 +40,8 @@ test('issue #49: multiple \'dc:title\' elements', async () => { const report = await ace('../data/issue49'); expect(report['earl:result']['earl:outcome']).toEqual('pass'); }); + +test.only('issue #53: XXX', async () => { + const report = await ace('../data/issue53'); + expect(report['earl:result']['earl:outcome']).toEqual('pass'); +}); diff --git a/tests/data/issue53/EPUB/content_001.xhtml b/tests/data/issue53/EPUB/content_001.xhtml new file mode 100644 index 00000000..d0e7d2ff --- /dev/null +++ b/tests/data/issue53/EPUB/content_001.xhtml @@ -0,0 +1,10 @@ + + +Minimal EPUB + + +

Loomings

+

Call me Ishmael.

+dummy + + diff --git a/tests/data/issue53/EPUB/content_002.xhtml b/tests/data/issue53/EPUB/content_002.xhtml new file mode 100644 index 00000000..d0e7d2ff --- /dev/null +++ b/tests/data/issue53/EPUB/content_002.xhtml @@ -0,0 +1,10 @@ + + +Minimal EPUB + + +

Loomings

+

Call me Ishmael.

+dummy + + diff --git a/tests/data/issue53/EPUB/image_001.jpg b/tests/data/issue53/EPUB/image_001.jpg new file mode 100644 index 00000000..d2c2e1ac --- /dev/null +++ b/tests/data/issue53/EPUB/image_001.jpg @@ -0,0 +1 @@ +fake image diff --git a/tests/data/issue53/EPUB/nav.xhtml b/tests/data/issue53/EPUB/nav.xhtml new file mode 100644 index 00000000..1d538c19 --- /dev/null +++ b/tests/data/issue53/EPUB/nav.xhtml @@ -0,0 +1,12 @@ + + +Minimal Nav + + + + + diff --git a/tests/data/issue53/EPUB/package.opf b/tests/data/issue53/EPUB/package.opf new file mode 100644 index 00000000..b94bf0e6 --- /dev/null +++ b/tests/data/issue53/EPUB/package.opf @@ -0,0 +1,27 @@ + + + + Minimal EPUB 3.0 + en + NOID + 2017-01-01T00:00:01Z + 2017-01-01T00:00:01Z + structuralNavigation + everything OK! + noFlashingHazard + noSoundHazard + noMotionSimulationHazard + textual + textual + + + + + + + + + + + + diff --git a/tests/data/issue53/META-INF/container.xml b/tests/data/issue53/META-INF/container.xml new file mode 100644 index 00000000..2cf00654 --- /dev/null +++ b/tests/data/issue53/META-INF/container.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/tests/data/issue53/mimetype b/tests/data/issue53/mimetype new file mode 100644 index 00000000..57ef03f2 --- /dev/null +++ b/tests/data/issue53/mimetype @@ -0,0 +1 @@ +application/epub+zip \ No newline at end of file