From 8f87004158b684a9cd70c110520c9d42f2a61ed5 Mon Sep 17 00:00:00 2001 From: adamjmcgrath Date: Wed, 14 Feb 2018 19:26:40 +0000 Subject: [PATCH 1/3] Remove merge-coverage and explicit parallel option --- README.md | 4 - index.js | 26 +++--- lib/attach-middleware.js | 85 +++++++++++-------- lib/coverage-merge.js | 60 ------------- package.json | 1 - test/fixtures/my-addon/config/coverage.js | 2 +- .../config/coverage.js | 2 +- .../config/coverage.js | 2 +- test/fixtures/my-app/config/coverage.js | 2 +- test/integration/app-coverage-test.js | 11 +-- test/unit/index-test.js | 34 ++++++-- tests/dummy/config/coverage-excludes.js | 3 +- tests/dummy/config/coverage-nested-folder.js | 2 +- tests/dummy/config/coverage-parallel.js | 5 -- 14 files changed, 103 insertions(+), 136 deletions(-) delete mode 100644 lib/coverage-merge.js delete mode 100644 tests/dummy/config/coverage-parallel.js diff --git a/README.md b/README.md index 9cdadf4a..a0fca859 100644 --- a/README.md +++ b/README.md @@ -30,8 +30,6 @@ and then: `cross-env COVERAGE=true ember test` -When running with `parallel` set to true, the final reports can be merged by using `ember coverage-merge`. The final merged output will be stored in the `coverageFolder`. - ## Configuration Configuration is optional. It should be put in a file at `config/coverage.js` (`configPath` configuration in package.json is honored). In addition to this you can configure Istanbul by adding a `.istanbul.yml` file to the root directory of your app (See https://github.com/gotwarlost/istanbul#configuring) @@ -46,8 +44,6 @@ Configuration is optional. It should be put in a file at `config/coverage.js` (` - `coverageFolder`: Defaults to `coverage`. A folder relative to the root of your project to store coverage results. -- `parallel`: Defaults to `false`. Should be set to true if parallel testing is being used, for example when using [ember-exam](https://github.com/trentmwillis/ember-exam) with the `--parallel` flag. This will generate the coverage reports in directories suffixed with `_` to avoid overwriting other threads reports. These reports can be joined by using the `ember coverage-merge` command (potentially as part of the [posttest hook](https://docs.npmjs.com/misc/scripts) in your `package.json`). - #### Example ```js module.exports = { diff --git a/index.js b/index.js index bd99eb08..59e996cd 100644 --- a/index.js +++ b/index.js @@ -3,7 +3,7 @@ var path = require('path'); var existsSync = require('exists-sync'); var fs = require('fs-extra'); -var attachMiddleware = require('./lib/attach-middleware'); +var { attachServerMiddleware, attachTestMiddleware } = require('./lib/attach-middleware'); var config = require('./lib/config'); const walkSync = require('walk-sync'); const VersionChecker = require('ember-cli-version-checker'); @@ -85,29 +85,35 @@ module.exports = { return undefined; }, - includedCommands: function() { - return { - 'coverage-merge': require('./lib/coverage-merge') - }; - }, - /** * If coverage is enabled attach coverage middleware to the express server run by ember-cli * @param {Object} startOptions - Express server start options */ serverMiddleware: function(startOptions) { - this.testemMiddleware(startOptions.app); + if (!this._isCoverageEnabled()) { + return; + } + attachServerMiddleware(startOptions.app, { + configPath: this.project.configPath(), + root: this.project.root, + fileLookup: this.fileLookup + }); }, testemMiddleware: function(app) { if (!this._isCoverageEnabled()) { return; } - attachMiddleware(app, { + const config = { configPath: this.project.configPath(), root: this.project.root, fileLookup: this.fileLookup - }); + }; + // if we're running `ember test --server` use the `serverMiddleware`. + if (process.argv.includes('--server') || process.argv.includes('-s')) { + return this.serverMiddleware({ app }, config); + } + attachTestMiddleware(app, config); }, // Custom Methods diff --git a/lib/attach-middleware.js b/lib/attach-middleware.js index 6f1dab01..743da914 100644 --- a/lib/attach-middleware.js +++ b/lib/attach-middleware.js @@ -1,10 +1,11 @@ 'use strict'; -var bodyParser = require('body-parser'); -var istanbul = require('istanbul-api'); -var getConfig = require('./config'); -var path = require('path'); -var crypto = require('crypto'); +const bodyParser = require('body-parser').json({ limit: '50mb' }); +const istanbul = require('istanbul-api'); +const getConfig = require('./config'); +const path = require('path'); + +const WRITE_COVERAGE = '/write-coverage'; function logError(err, req, res, next) { console.error(err.stack); @@ -16,40 +17,52 @@ function fixFilePaths(coverageData, fileLookup) { return coverageData; } -module.exports = function(app, options) { - app.post('/write-coverage', - bodyParser.json({ limit: '50mb' }), - function(req, res) { - var config = getConfig(options.configPath); - - if (config.parallel) { - config.coverageFolder = config.coverageFolder + '_' + crypto.randomBytes(4).toString('hex'); - if (config.reporters.indexOf('json') === -1) { - config.reporters.push('json'); - } - } - - if (config.reporters.indexOf('json-summary') === -1) { - config.reporters.push('json-summary'); - } - - let reporter = istanbul.createReporter(); - if (config.coverageFolder) { - reporter.dir = path.join(options.root, config.coverageFolder); - } - let map = istanbul.libCoverage.createCoverageMap(); - let coverage = req.body; +function writeCoverage(coverage, fileLookup, map) { + Object.keys(fileLookup).forEach(filename => { + let fileCoverage = coverage[filename] || istanbul.libCoverage.createFileCoverage(filename).data; + map.addFileCoverage(fixFilePaths(fileCoverage, fileLookup)); + }); +} - Object.keys(options.fileLookup).forEach(filename => { - let fileCoverage = coverage[filename] || istanbul.libCoverage.createFileCoverage(filename).data; - map.addFileCoverage(fixFilePaths(fileCoverage, options.fileLookup)); - }); +function reportCoverage(map, root, configPath) { + let config = getConfig(configPath); + let reporter = istanbul.createReporter(); + if (config.coverageFolder) { + reporter.dir = path.join(root, config.coverageFolder); + } + reporter.addAll(config.reporters); + reporter.write(map); +} - reporter.addAll(config.reporters); - reporter.write(map); +function coverageHandler(map, options, req, res) { + writeCoverage(req.body, options.fileLookup, map); + reportCoverage(map, options.root, options.configPath); + res.send(map.getCoverageSummary()); +} - const results = map.getCoverageSummary(); - res.send(results); +// Used when app is in dev mode (`ember serve`). +// Creates a new coverage map on every request. +const attachServerMiddleware = function(app, options) { + app.post(WRITE_COVERAGE, + bodyParser, + (...args) => { + let map = istanbul.libCoverage.createCoverageMap(); + coverageHandler(map, options, ...args); }, logError); }; + +// Used when app is in ci mode (`ember test`). +// Collects the coverage on each request and merges it into the coverage map. +const attachTestMiddleware = function(app, options) { + let map = istanbul.libCoverage.createCoverageMap(); + app.post(WRITE_COVERAGE, + bodyParser, + coverageHandler.bind(null, map, options), + logError); +}; + +module.exports = { + attachServerMiddleware, + attachTestMiddleware, +}; diff --git a/lib/coverage-merge.js b/lib/coverage-merge.js deleted file mode 100644 index 18186376..00000000 --- a/lib/coverage-merge.js +++ /dev/null @@ -1,60 +0,0 @@ -'use strict'; - -var path = require('path'); -var getConfig = require('./config'); -var dir = require('node-dir'); -var Promise = require('rsvp').Promise; - -/** - * Merge together coverage files created when running in multiple threads, - * for example when being used with ember exam and parallel runs. - */ -module.exports = { - name: 'coverage-merge', - description: 'Merge multiple coverage files together.', - run: function () { - var istanbul = require('istanbul-api'); - var config = this._getConfig(); - - var coverageFolderSplit = config.coverageFolder.split('/'); - var coverageFolder = coverageFolderSplit.pop(); - var coverageRoot = this.project.root + '/' + coverageFolderSplit.join('/'); - var coverageDirRegex = new RegExp(coverageFolder + '_.*'); - - let reporter = istanbul.createReporter(); - reporter.dir = path.join(coverageRoot, coverageFolder); - let map = istanbul.libCoverage.createCoverageMap(); - - return new Promise(function (resolve, reject) { - dir.readFiles(coverageRoot, { matchDir: coverageDirRegex, match: /coverage-final\.json/ }, - function (err, coverageSummary, next) { - if (err) { - reject(err); - } - map.merge(JSON.parse(coverageSummary)); - next(); - }, - function (err) { - if (err) { - reject(err); - } - - if (config.reporters.indexOf('json-summary') === -1) { - config.reporters.push('json-summary'); - } - - reporter.addAll(config.reporters); - reporter.write(map); - resolve(); - }); - }); - }, - - /** - * Get project configuration - * @returns {Configuration} project configuration - */ - _getConfig: function () { - return getConfig(this.project.configPath()); - } -}; diff --git a/package.json b/package.json index 1c58573d..e6a2a81b 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,6 @@ "fs-extra": "^5.0.0", "istanbul-api": "^1.1.14", "lodash.concat": "^4.5.0", - "node-dir": "^0.1.16", "rsvp": "^4.8.1", "walk-sync": "^0.3.2" }, diff --git a/test/fixtures/my-addon/config/coverage.js b/test/fixtures/my-addon/config/coverage.js index 8e9ccaf6..e3724c51 100644 --- a/test/fixtures/my-addon/config/coverage.js +++ b/test/fixtures/my-addon/config/coverage.js @@ -1,3 +1,3 @@ module.exports = { - reporters: ['lcov', 'html', 'text'] + reporters: ['lcov', 'html', 'text', 'json-summary'] }; diff --git a/test/fixtures/my-app-with-in-repo-addon/config/coverage.js b/test/fixtures/my-app-with-in-repo-addon/config/coverage.js index 8e9ccaf6..e3724c51 100644 --- a/test/fixtures/my-app-with-in-repo-addon/config/coverage.js +++ b/test/fixtures/my-app-with-in-repo-addon/config/coverage.js @@ -1,3 +1,3 @@ module.exports = { - reporters: ['lcov', 'html', 'text'] + reporters: ['lcov', 'html', 'text', 'json-summary'] }; diff --git a/test/fixtures/my-app-with-in-repo-engine/config/coverage.js b/test/fixtures/my-app-with-in-repo-engine/config/coverage.js index 8e9ccaf6..e3724c51 100644 --- a/test/fixtures/my-app-with-in-repo-engine/config/coverage.js +++ b/test/fixtures/my-app-with-in-repo-engine/config/coverage.js @@ -1,3 +1,3 @@ module.exports = { - reporters: ['lcov', 'html', 'text'] + reporters: ['lcov', 'html', 'text', 'json-summary'] }; diff --git a/test/fixtures/my-app/config/coverage.js b/test/fixtures/my-app/config/coverage.js index 8e9ccaf6..e3724c51 100644 --- a/test/fixtures/my-app/config/coverage.js +++ b/test/fixtures/my-app/config/coverage.js @@ -1,3 +1,3 @@ module.exports = { - reporters: ['lcov', 'html', 'text'] + reporters: ['lcov', 'html', 'text', 'json-summary'] }; diff --git a/test/integration/app-coverage-test.js b/test/integration/app-coverage-test.js index 57a7fb5f..02ed0b17 100644 --- a/test/integration/app-coverage-test.js +++ b/test/integration/app-coverage-test.js @@ -76,14 +76,10 @@ describe('app coverage generation', function() { }); }); - it('uses parallel configuration and merges coverage when merge-coverage command is issued', function() { + it('merges coverage when tests are run in parallel', function() { expect(dir(`${app.path}/coverage`)).to.not.exist; - fs.copySync('tests/dummy/config/coverage-parallel.js', `${app.path}/config/coverage.js`); process.env.COVERAGE = true; return app.run('ember', 'exam', '--split=2', '--parallel=true').then(function() { - expect(dir(`${app.path}/coverage`)).to.not.exist; - return app.run('ember', 'coverage-merge'); - }).then(function() { expect(file(`${app.path}/coverage/lcov-report/index.html`)).to.not.be.empty; expect(file(`${app.path}/coverage/index.html`)).to.not.be.empty; var summary = fs.readJSONSync(`${app.path}/coverage/coverage-summary.json`); @@ -91,16 +87,13 @@ describe('app coverage generation', function() { }); }); - it('uses nested coverageFolder and parallel configuration and run merge-coverage', function() { + it('uses nested coverageFolder', function() { var coverageFolder = `${app.path}/coverage/abc/easy-as/123`; expect(dir(coverageFolder)).to.not.exist; fs.copySync('tests/dummy/config/coverage-nested-folder.js', `${app.path}/config/coverage.js`); process.env.COVERAGE = true; return app.run('ember', 'exam', '--split=2', '--parallel=true').then(function() { - expect(dir(coverageFolder)).to.not.exist; - return app.run('ember', 'coverage-merge'); - }).then(function() { expect(file(`${coverageFolder}/lcov-report/index.html`)).to.not.be.empty; expect(file(`${coverageFolder}/index.html`)).to.not.be.empty; var summary = fs.readJSONSync(`${coverageFolder}/coverage-summary.json`); diff --git a/test/unit/index-test.js b/test/unit/index-test.js index 3266fe62..7d096f95 100644 --- a/test/unit/index-test.js +++ b/test/unit/index-test.js @@ -65,15 +65,39 @@ describe('index.js', function() { }); describe('serverMiddleware', function() { + var app; + beforeEach(function() { - sandbox.stub(Index, 'testemMiddleware'); - Index.serverMiddleware({ - app: 'foo-bar' + app = { + post: sinon.spy() + }; + + sandbox.stub(Index, 'project').value({ + root: '/path/to/foo-bar', + configPath: sinon.stub().returns('tests/dummy/config/environment.js') }); }); - it('calls testemMiddleware with correct arguments', function() { - expect(Index.testemMiddleware.lastCall.args).to.eql(['foo-bar']); + describe('when coverage is enabled', function() { + beforeEach(function() { + sandbox.stub(Index, '_isCoverageEnabled').returns(true); + Index.serverMiddleware({ app }); + }); + + it('adds POST endpoint to app', function() { + expect(app.post.callCount).to.equal(1); + }); + }); + + describe('when coverage is not enabled', function() { + beforeEach(function() { + sandbox.stub(Index, '_isCoverageEnabled').returns(false); + Index.serverMiddleware({ app }); + }); + + it('does not add POST endpoint to app', function() { + expect(app.post.callCount).to.equal(0); + }); }); }); diff --git a/tests/dummy/config/coverage-excludes.js b/tests/dummy/config/coverage-excludes.js index e84a75d7..462c37b2 100644 --- a/tests/dummy/config/coverage-excludes.js +++ b/tests/dummy/config/coverage-excludes.js @@ -3,5 +3,6 @@ module.exports = { excludes: [ '**/utils/my-uncovered-util.js' - ] + ], + reporters: ['lcov', 'html', 'text', 'json-summary'] }; diff --git a/tests/dummy/config/coverage-nested-folder.js b/tests/dummy/config/coverage-nested-folder.js index ae5fac98..446eaa4f 100644 --- a/tests/dummy/config/coverage-nested-folder.js +++ b/tests/dummy/config/coverage-nested-folder.js @@ -3,5 +3,5 @@ module.exports = { coverageFolder: 'coverage/abc/easy-as/123', - parallel: true + reporters: ['lcov', 'html', 'text', 'json-summary'] }; diff --git a/tests/dummy/config/coverage-parallel.js b/tests/dummy/config/coverage-parallel.js deleted file mode 100644 index a33514b6..00000000 --- a/tests/dummy/config/coverage-parallel.js +++ /dev/null @@ -1,5 +0,0 @@ -/* eslint-env node */ - -module.exports = { - parallel: true -}; From a973cfe2f003aa8ae7e023ad6f3de61de4ba1aca Mon Sep 17 00:00:00 2001 From: adamjmcgrath Date: Wed, 14 Feb 2018 20:20:59 +0000 Subject: [PATCH 2/3] fix lint --- index.js | 6 +++--- lib/attach-middleware.js | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/index.js b/index.js index 59e996cd..e70a454d 100644 --- a/index.js +++ b/index.js @@ -3,7 +3,7 @@ var path = require('path'); var existsSync = require('exists-sync'); var fs = require('fs-extra'); -var { attachServerMiddleware, attachTestMiddleware } = require('./lib/attach-middleware'); +var attachMiddleware = require('./lib/attach-middleware'); var config = require('./lib/config'); const walkSync = require('walk-sync'); const VersionChecker = require('ember-cli-version-checker'); @@ -93,7 +93,7 @@ module.exports = { if (!this._isCoverageEnabled()) { return; } - attachServerMiddleware(startOptions.app, { + attachMiddleware.serverMiddleware(startOptions.app, { configPath: this.project.configPath(), root: this.project.root, fileLookup: this.fileLookup @@ -113,7 +113,7 @@ module.exports = { if (process.argv.includes('--server') || process.argv.includes('-s')) { return this.serverMiddleware({ app }, config); } - attachTestMiddleware(app, config); + attachMiddleware.testMiddleware(app, config); }, // Custom Methods diff --git a/lib/attach-middleware.js b/lib/attach-middleware.js index 743da914..af6c96ff 100644 --- a/lib/attach-middleware.js +++ b/lib/attach-middleware.js @@ -42,19 +42,19 @@ function coverageHandler(map, options, req, res) { // Used when app is in dev mode (`ember serve`). // Creates a new coverage map on every request. -const attachServerMiddleware = function(app, options) { +const serverMiddleware = function(app, options) { app.post(WRITE_COVERAGE, bodyParser, - (...args) => { + (req, res) => { let map = istanbul.libCoverage.createCoverageMap(); - coverageHandler(map, options, ...args); + coverageHandler(map, options, req, res); }, logError); }; // Used when app is in ci mode (`ember test`). // Collects the coverage on each request and merges it into the coverage map. -const attachTestMiddleware = function(app, options) { +const testMiddleware = function(app, options) { let map = istanbul.libCoverage.createCoverageMap(); app.post(WRITE_COVERAGE, bodyParser, @@ -63,6 +63,6 @@ const attachTestMiddleware = function(app, options) { }; module.exports = { - attachServerMiddleware, - attachTestMiddleware, + serverMiddleware, + testMiddleware, }; From 5b0dd863edd5c25274016bd4d23d55e2ffd9ad1a Mon Sep 17 00:00:00 2001 From: Adam Mcgrath Date: Tue, 20 Feb 2018 23:37:01 +0000 Subject: [PATCH 3/3] Update README.md Describe how parallel works --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index a0fca859..71c924ca 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,10 @@ and then: `cross-env COVERAGE=true ember test` +Coverage also works when running tests in parallel, eg: + +`COVERAGE=true ember exam --split=2 --parallel=true` + ## Configuration Configuration is optional. It should be put in a file at `config/coverage.js` (`configPath` configuration in package.json is honored). In addition to this you can configure Istanbul by adding a `.istanbul.yml` file to the root directory of your app (See https://github.com/gotwarlost/istanbul#configuring)