Skip to content

Commit

Permalink
fix: enable successive calls in the same process
Browse files Browse the repository at this point in the history
The `report` module was using a shared report object which was
mutated all along the checking process. When calling `ace.js`
multiple times sequentially, that shared report object wasn't
cleaned and the later runs inherited the assertions from the
previous runs.

The `report` module now exports a single `EPUB` class, with
no shared state. The report is passed along the checking process,
up until the `checkSingle` method. After all documents are checked,
we consolidate the individual results and add them to the given
report object.

Also add some integration tests for checking the JSON report.
  • Loading branch information
rdeltour committed Oct 2, 2017
1 parent 575530c commit 7461fec
Show file tree
Hide file tree
Showing 8 changed files with 171 additions and 110 deletions.
11 changes: 4 additions & 7 deletions src/checker/checker-nightmare.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const fs = require('fs');
const path = require('path');
const Nightmare = require('nightmare');
const report = require('../report/report.js');
const axe2ace = require('../report/axe2ace.js');
const winston = require('winston');

Expand Down Expand Up @@ -58,12 +57,10 @@ function checkSingle(spineItem, epub, nightmare) {
const results = json;
results.assertions = (json.axe != null) ? axe2ace.axe2ace(spineItem, json.axe) : [];
delete results.axe;
var numIssues = 0;
if (results.assertions != null) {
numIssues = results.assertions.assertions.length;
report.addContentDocAssertion(results.assertions);
}
winston.info(`- ${numIssues} issues found`);
winston.info(`- ${
(results.assertions == null)
? 'No'
: results.assertions.assertions.length} issues found`);
if (results.data != null && results.data.images != null) {
results.data.images.forEach((img) => {
const imageFullPath = path.resolve(path.dirname(spineItem.filepath), img.path);
Expand Down
23 changes: 15 additions & 8 deletions src/checker/checker.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,31 @@
'use strict';

const checker = require('./checker-nightmare.js');
const report = require('../report/report.js');
const winston = require('winston');

function finalize(results) {
function finalize(results, report) {
// Copy assertions
results
.filter(res => res.assertions != null)
.forEach(res => report.addContentDocAssertion(res.assertions));
// Get a flat array of all the headings in the documents
const headings = [].concat(...results.map(single => single.outlines.headings));
const headings = []
.concat(...results.map(docResult => docResult.outlines.headings))
.filter(e => e !== undefined);
report.addHeadings(headings);
// Aggregated array of the HTML outlines
const htmlOutlines = results.map(single => single.outlines.html);
const htmlOutlines = results.map(docResult => docResult.outlines.html);
report.addHTMLOutlines(htmlOutlines);
// Get a flat array of the extracted images
const images = [].concat(...results.map(single => single.data.images));
const images = []
.concat(...results.map(docResult => docResult.data.images))
.filter(e => e !== undefined);
report.addImages(images);
return results;
return report;
}

module.exports.check = function check(epub) {
module.exports.check = function check(epub, report) {
winston.info('Checking documents...');
return checker.check(epub)
.then(results => finalize(results));
.then(results => finalize(results, report));
};
36 changes: 11 additions & 25 deletions src/core/ace.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const tmp = require('tmp');

const checker = require('../checker/checker.js');
const EPUB = require('../epub/epub.js');
const report = require('../report/report.js');
const Report = require('../report/report.js');
const winston = require('winston');

tmp.setGracefulCleanup();
Expand Down Expand Up @@ -53,22 +53,21 @@ module.exports = function ace(epubPath, options) {
epub.extract()
.then(() => epub.parse())
// initialize the report
.then(() => report.initialize(epub))
.then(() => new Report(epub))
// Report the Nav Doc
.then(() => report.addEPUBNav(epub.navDoc))
.then(report => report.addEPUBNav(epub.navDoc))
// Check each Content Doc
.then(() => checker.check(epub))
.then(report => checker.check(epub, report))
// Process the Results
.then(() => {
.then((report) => {
if (options.outdir === undefined) {
winston.info(JSON.stringify(report.getJsonReport(), null, ' '));
} else {
return Promise.all([
report.copyData(options.outdir),
report.saveJson(options.outdir),
report.saveHtml(options.outdir),
]);
return winston.info(JSON.stringify(report.json, null, ' '));
}
return Promise.all([
report.copyData(options.outdir),
report.saveJson(options.outdir),
report.saveHtml(options.outdir),
]);
})
.then(() => {
winston.info('Done.');
Expand All @@ -80,17 +79,4 @@ module.exports = function ace(epubPath, options) {
reject(jobId);
});
});
// // Alternatively: use reduce to process results progressively ?
// new EPUB(epubPath).extract()
// .then(epub => epub.getContentDocsPaths())
// .then(paths => paths.map(path => `file://${path}`))
// .then(urls =>
// urls.reduce((sequence, url) =>
// sequence.then(() => check(url)).then((result) => {
// console.log(`do sth with ${result}`);
// return result;
// }), Promise.resolve()))
// .then(...);
// getJSON('story.json').then(function(story) {
// addHtmlToPage(story.heading);
};
1 change: 0 additions & 1 deletion src/report/json-report-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ function withAssertion(obj, assertion) {
}
obj["assertions"].push(assertion);
withResult(obj, calculateResult(obj.assertions));

return obj;
}
function withResult(obj, result) {
Expand Down
99 changes: 52 additions & 47 deletions src/report/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ const winston = require('winston');
const reportBuilder = require('./json-report-builder.js');
const a11yMetaChecker = require("./analyze-a11y-metadata.js");

const report = exports;
const jsonReport = new reportBuilder.Report();

function headingsToOutline(headings) {
const result = [];
Expand Down Expand Up @@ -57,45 +55,51 @@ function aggregateHTMLOutlines(outlines) {
return `<ol><li>${outlines.join('</li><li>')}</li></ol>`;
}

module.exports = class Report {

report.initialize = function initialize(epub) {
jsonReport.withTitle("ACE Report")

constructor(epub) {
this.json = new reportBuilder.Report();
this.json.withTitle("ACE Report")
.withDescription("Accessibility Checker Report" )
.withTestSubject(epub.path, "", "", epub.metadata);
jsonReport.withA11yMeta(a11yMetaChecker.analyze(epub.metadata));
this.json.withA11yMeta(a11yMetaChecker.analyze(epub.metadata));
}

}
report.addContentDocAssertion = function addContentDocAssertion(assertion) {
jsonReport.withAssertion(assertion);
}
report.addOutline = function addOutline(outline) {
jsonReport.withHOutline(outline);
}
report.addHeadings = function addHeadings(headings) {
jsonReport.withHeadingsOutline(headingsToOutline(headings));
}
report.addHTMLOutlines = function addHTMLOutlines(outlines) {
jsonReport.withHTMLOutline(aggregateHTMLOutlines(outlines));
}
report.addEPUBNav = function addEPUBNav(navDoc) {
jsonReport.withEPUBOutline(navDoc.tocHTML);
}
report.addImages = function addHImages(images) {
jsonReport.withImages(images);
}
report.getJsonReport = function getJsonReport() {
return jsonReport;
}
report.copyData = function copyData(outdir) {
addContentDocAssertion(assertion) {
this.json.withAssertion(assertion);
return this;
}
addOutline(outline) {
this.json.withHOutline(outline);
return this;
}
addHeadings(headings) {
this.json.withHeadingsOutline(headingsToOutline(headings));
return this;
}
addHTMLOutlines(outlines) {
this.json.withHTMLOutline(aggregateHTMLOutlines(outlines));
return this;
}
addEPUBNav(navDoc) {
this.json.withEPUBOutline(navDoc.tocHTML);
return this;
}
addImages(images) {
this.json.withImages(images);
return this;
}
copyData(outdir) {
winston.info("Copying data");
if (jsonReport.data === null) return;
if (this.json.data === null) return;
return fs.pathExists(outdir)
.then((exists) => {
if (!exists) fs.mkdirSync(outdir);
})
.then(() => {
if (jsonReport.data.images != null) {
jsonReport.data.images.forEach((img) => {
if (this.json.data.images != null) {
this.json.data.images.forEach((img) => {
const fromPath = img.filepath;
const toPath = path.join(outdir, 'data', img.path);
delete img.filepath;
Expand All @@ -105,31 +109,32 @@ report.copyData = function copyData(outdir) {
});
}
});
}
report.saveJson = function saveJson(outdir) {
}
saveJson(outdir) {
winston.info("Saving JSON report");
return fs.pathExists(outdir)
.then((exists) => {
if (!exists) fs.mkdirSync(outdir);
})
.then(() => {
const aceReport = JSON.stringify(jsonReport, null, ' ');
const aceReport = JSON.stringify(this.json, null, ' ');
return aceReport;
})
.then((aceReport) => Promise.all([
fs.writeFile(path.join(outdir, 'ace.json'), aceReport, 'UTF-8'),
]));
}
report.saveHtml = function saveHtml(outdir) {
winston.info("Saving HTML report");
// create a js file that the html report uses as its data source
const aceReport = JSON.stringify(jsonReport, null, ' ');
const js = "const aceReportData = " + aceReport + ";";

// copy report.html and the contents of /js and /css to the outdir
return fs.copy(path.join(__dirname, 'resources/report.html'), path.join(outdir, "report.html"))
//.then(() => fs.copy(path.join(__dirname, './resources/css/'), path.join(outdir, "css/")))
.then(() => fs.copy(path.join(__dirname, './resources/js/'), path.join(outdir, "js/")))
.then(() => fs.writeFile(path.join(outdir, 'js/', 'aceReportData.js'), js, 'UTF-8'))
.catch(err => winston.error(err));
}
saveHtml(outdir) {
winston.info("Saving HTML report");
// create a js file that the html report uses as its data source
const aceReport = JSON.stringify(this.json, null, ' ');
const js = "const aceReportData = " + aceReport + ";";

// copy report.html and the contents of /js and /css to the outdir
return fs.copy(path.join(__dirname, 'resources/report.html'), path.join(outdir, "report.html"))
//.then(() => fs.copy(path.join(__dirname, './resources/css/'), path.join(outdir, "css/")))
.then(() => fs.copy(path.join(__dirname, './resources/js/'), path.join(outdir, "js/")))
.then(() => fs.writeFile(path.join(outdir, 'js/', 'aceReportData.js'), js, 'UTF-8'))
.catch(err => winston.error(err));
}
}
35 changes: 13 additions & 22 deletions tests/__tests__/report_files.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,19 @@ const fs = require('fs');
const path = require('path');
const tmp = require('tmp');

const ace = require('../../src/core/ace.js');
const runAce = require('../runAceJS');

tmp.setGracefulCleanup();

let outdir;
let tmpdir;

tmp.setGracefulCleanup();
const ace = function ace(epub, options = {}) {
return runAce(epub, Object.assign({
outdir: outdir.name,
tmp: tmpdir.name,
}, options));
};

beforeEach(() => {
outdir = tmp.dirSync({ prefix: 'ace_out_', unsafeCleanup: true });
Expand All @@ -21,31 +28,15 @@ afterEach(() => {
tmpdir.removeCallback();
});

function runAce(epub, {
cwd = process.cwd(),
outpath = outdir.name,
tmppath = tmpdir.name,
verbose = false,
silent = true,
} = {}) {
return ace(epub, {
cwd,
outdir: outpath,
tmpdir: tmppath,
verbose,
silent,
});
}

test('unexisting EPUB fails with an error', () => {
expect.assertions(1);
return expect(runAce('noepub'))
return expect(ace('noepub'))
.rejects.toMatch('');
});

test('packaged EPUB with absolute path', async () => {
expect.assertions(1);
await runAce(path.join(__dirname, '../data/base-epub-30.epub'));
await ace(path.join(__dirname, '../data/base-epub-30.epub'));
expect(fs.existsSync(path.join(outdir.name, 'report.html'))).toBeTruthy();
});

Expand All @@ -57,7 +48,7 @@ test('packaged EPUB with relative path', async () => {

test('expanded EPUB with absolute path', async () => {
expect.assertions(1);
await runAce(path.join(__dirname, '../data/base-epub-30'));
await ace(path.join(__dirname, '../data/base-epub-30'));
expect(fs.existsSync(path.join(outdir.name, 'report.html'))).toBeTruthy();
});

Expand All @@ -72,7 +63,7 @@ test('files don’t leak outside the report dir', async () => {
const outpath = path.join(outdir.name, 'report');
fs.mkdirSync(outpath);
expect.assertions(2);
await runAce(path.join(__dirname, '../data/issue33'), { outpath });
await ace(path.join(__dirname, '../data/issue33'), { outdir: outpath });
expect(fs.existsSync(path.join(outpath, 'report.html'))).toBeTruthy();
expect(fs.existsSync(path.join(outpath, 'data/EPUB/images/img_001.jpg'))).toBeTruthy();
});
Loading

0 comments on commit 7461fec

Please sign in to comment.