Skip to content

Commit

Permalink
chore(doclint): remove SourceFactory (puppeteer#2447)
Browse files Browse the repository at this point in the history 
SourceFactory was meant to cache Sources so that they could be used
in different preprocessor tasks.

This turned out to be over-engineering. This patch kills the layer.
  • Loading branch information
@aslushnikov
aslushnikov authored Apr 26, 2018
1 parent 6d19db4 commit 13a4149
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 66 deletions.
50 changes: 8 additions & 42 deletions utils/doclint/SourceFactory.js → utils/doclint/Source.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,66 +82,33 @@ class Source {
hasUpdatedText() {
return this._hasUpdatedText;
}
}

class SourceFactory {
constructor() {
this._sources = new Map();
}

/**
* @return {!Array<!Source>}
*/
sources() {
return Array.from(this._sources.values());
}

/**
* @return {!Promise<boolean>}
*/
async saveChangedSources() {
const changedSources = Array.from(this._sources.values()).filter(source => source.hasUpdatedText());
if (!changedSources.length)
return false;
await Promise.all(changedSources.map(source => writeFileAsync(source.filePath(), source.text())));
return true;
async save() {
await writeFileAsync(this.filePath(), this.text());
}

/**
* @param {string} filePath
* @return {!Promise<Source>}
*/
async readFile(filePath) {
static async readFile(filePath) {
filePath = path.resolve(filePath);
let source = this._sources.get(filePath);
if (!source) {
const text = await readFileAsync(filePath, {encoding: 'utf8'});
source = new Source(filePath, text);
this._sources.set(filePath, source);
}
return source;
const text = await readFileAsync(filePath, {encoding: 'utf8'});
return new Source(filePath, text);
}

/**
* @param {string} dirPath
* @param {string=} extension
* @return {!Promise<!Array<!Source>>}
*/
async readdir(dirPath, extension = '') {
static async readdir(dirPath, extension = '') {
const fileNames = await readdirAsync(dirPath);
const filePaths = fileNames.filter(fileName => fileName.endsWith(extension)).map(fileName => path.join(dirPath, fileName));
return Promise.all(filePaths.map(filePath => this.readFile(filePath)));
}

/**
* @param {string} filePath
* @param {string} text
* @return {!Source}
*/
createForTest(filePath, text) {
return new Source(filePath, text);
return Promise.all(filePaths.map(filePath => Source.readFile(filePath)));
}
}
module.exports = Source;

/**
* @param {function(?)} nodeFunction
Expand All @@ -166,4 +133,3 @@ function promisify(nodeFunction) {
};
}

module.exports = SourceFactory;
13 changes: 5 additions & 8 deletions utils/doclint/check_public_api/test/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
const path = require('path');
const puppeteer = require('../../../..');
const checkPublicAPI = require('..');
const SourceFactory = require('../../SourceFactory');
const Source = require('../../Source');
const mdBuilder = require('../MDBuilder');
const jsBuilder = require('../JSBuilder');
const GoldenUtils = require('../../../../test/golden-utils');
Expand Down Expand Up @@ -64,9 +64,8 @@ async function testLint(state, test) {
toBeGolden: GoldenUtils.compare.bind(null, dirPath, dirPath)
});

const factory = new SourceFactory();
const mdSources = await factory.readdir(dirPath, '.md');
const jsSources = await factory.readdir(dirPath, '.js');
const mdSources = await Source.readdir(dirPath, '.md');
const jsSources = await Source.readdir(dirPath, '.js');
const messages = await checkPublicAPI(page, mdSources, jsSources);
const errors = messages.map(message => message.text);
expect(errors.join('\n')).toBeGolden('result.txt');
Expand All @@ -77,8 +76,7 @@ async function testMDBuilder(state, test) {
const {expect} = new Matchers({
toBeGolden: GoldenUtils.compare.bind(null, dirPath, dirPath)
});
const factory = new SourceFactory();
const sources = await factory.readdir(dirPath, '.md');
const sources = await Source.readdir(dirPath, '.md');
const {documentation} = await mdBuilder(page, sources);
expect(serialize(documentation)).toBeGolden('result.txt');
}
Expand All @@ -88,8 +86,7 @@ async function testJSBuilder(state, test) {
const {expect} = new Matchers({
toBeGolden: GoldenUtils.compare.bind(null, dirPath, dirPath)
});
const factory = new SourceFactory();
const sources = await factory.readdir(dirPath, '.js');
const sources = await Source.readdir(dirPath, '.js');
const {documentation} = await jsBuilder(sources);
expect(serialize(documentation)).toBeGolden('result.txt');
}
Expand Down
17 changes: 12 additions & 5 deletions utils/doclint/cli.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

const puppeteer = require('../..');
const path = require('path');
const SourceFactory = require('./SourceFactory');
const Source = require('./Source');

const PROJECT_DIR = path.join(__dirname, '..', '..');
const VERSION = require(path.join(PROJECT_DIR, 'package.json')).version;
Expand All @@ -31,13 +31,13 @@ run();
async function run() {
const startTime = Date.now();

const sourceFactory = new SourceFactory();
/** @type {!Array<!Message>} */
const messages = [];
let changedFiles = false;

// Documentation checks.
{
const apiSource = await sourceFactory.readFile(path.join(PROJECT_DIR, 'docs', 'api.md'));
const apiSource = await Source.readFile(path.join(PROJECT_DIR, 'docs', 'api.md'));
const mdSources = [apiSource];

const toc = require('./toc');
Expand All @@ -49,9 +49,16 @@ async function run() {
const browser = await puppeteer.launch({args: ['--no-sandbox']});
const page = await browser.newPage();
const checkPublicAPI = require('./check_public_api');
const jsSources = await sourceFactory.readdir(path.join(PROJECT_DIR, 'lib'), '.js');
const jsSources = await Source.readdir(path.join(PROJECT_DIR, 'lib'), '.js');
messages.push(...await checkPublicAPI(page, mdSources, jsSources));
await browser.close();

for (const source of mdSources) {
if (!source.hasUpdatedText())
continue;
await source.save();
changedFiles = true;
}
}

// Report results.
Expand All @@ -74,7 +81,7 @@ async function run() {
}
}
let clearExit = messages.length === 0;
if (await sourceFactory.saveChangedSources()) {
if (changedFiles) {
if (clearExit)
console.log(`${YELLOW_COLOR}Some files were updated.${RESET_COLOR}`);
clearExit = false;
Expand Down
20 changes: 9 additions & 11 deletions utils/doclint/preprocessor/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@
*/

const preprocessor = require('.');
const SourceFactory = require('../SourceFactory');
const factory = new SourceFactory();

const Source = require('../Source');
const {TestRunner, Reporter, Matchers} = require('../../testrunner/');
const runner = new TestRunner();
new Reporter(runner);
Expand All @@ -29,7 +27,7 @@ const {expect} = new Matchers();

describe('preprocessor', function() {
it('should throw for unknown command', function() {
const source = factory.createForTest('doc.md', `
const source = new Source('doc.md', `
<!-- gen:unknown-command -->something<!-- gen:stop -->
`);
const messages = preprocessor([source], '1.1.1');
Expand All @@ -40,7 +38,7 @@ describe('preprocessor', function() {
});
describe('gen:version', function() {
it('should work', function() {
const source = factory.createForTest('doc.md', `
const source = new Source('doc.md', `
Puppeteer <!-- gen:version -->XXX<!-- gen:stop -->
`);
const messages = preprocessor([source], '1.2.0');
Expand All @@ -52,7 +50,7 @@ describe('preprocessor', function() {
`);
});
it('should work for *-post versions', function() {
const source = factory.createForTest('doc.md', `
const source = new Source('doc.md', `
Puppeteer <!-- gen:version -->XXX<!-- gen:stop -->
`);
const messages = preprocessor([source], '1.2.0-post');
Expand All @@ -64,13 +62,13 @@ describe('preprocessor', function() {
`);
});
it('should tolerate different writing', function() {
const source = factory.createForTest('doc.md', `Puppeteer v<!-- gEn:version -->WHAT
const source = new Source('doc.md', `Puppeteer v<!-- gEn:version -->WHAT
<!-- GEN:stop -->`);
preprocessor([source], '1.1.1');
expect(source.text()).toBe(`Puppeteer v<!-- gEn:version -->v1.1.1<!-- GEN:stop -->`);
});
it('should not tolerate missing gen:stop', function() {
const source = factory.createForTest('doc.md', `<!--GEN:version-->`);
const source = new Source('doc.md', `<!--GEN:version-->`);
const messages = preprocessor([source], '1.2.0');
expect(source.hasUpdatedText()).toBe(false);
expect(messages.length).toBe(1);
Expand All @@ -80,7 +78,7 @@ describe('preprocessor', function() {
});
describe('gen:empty-if-release', function() {
it('should clear text when release version', function() {
const source = factory.createForTest('doc.md', `
const source = new Source('doc.md', `
<!-- gen:empty-if-release -->XXX<!-- gen:stop -->
`);
const messages = preprocessor([source], '1.1.1');
Expand All @@ -92,7 +90,7 @@ describe('preprocessor', function() {
`);
});
it('should keep text when non-release version', function() {
const source = factory.createForTest('doc.md', `
const source = new Source('doc.md', `
<!-- gen:empty-if-release -->XXX<!-- gen:stop -->
`);
const messages = preprocessor([source], '1.1.1-post');
Expand All @@ -103,7 +101,7 @@ describe('preprocessor', function() {
});
});
it('should work with multiple commands', function() {
const source = factory.createForTest('doc.md', `
const source = new Source('doc.md', `
<!-- gen:version -->XXX<!-- gen:stop -->
<!-- gen:empty-if-release -->YYY<!-- gen:stop -->
<!-- gen:version -->ZZZ<!-- gen:stop -->
Expand Down

0 comments on commit 13a4149

Please sign in to comment.