Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support egg-bin test --changed #111

Merged
merged 2 commits into from
Sep 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ You can pass any mocha argv.
- `--timeout` milliseconds, default to 30000
- `--full-trace` display the full stack trace, default to false.
- `--typescript` / `--ts` enable typescript support, default to `false`.
- `--changed` / `-c` only test changed test files(test files means files that match `${pwd}/test/**/*.test.(js|ts)`)
- see more at https://mochajs.org/#usage

#### environment
Expand Down
6 changes: 3 additions & 3 deletions lib/cmd/cov.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class CovCommand extends Command {
};

// save coverage-xxxx.json to $PWD/coverage
const covArgs = this.getCovArgs(context);
const covArgs = yield this.getCovArgs(context);
if (!covArgs) return;
debug('covArgs: %j', covArgs);
yield this.helper.forkNode(nycCli, covArgs, opt);
Expand All @@ -104,7 +104,7 @@ class CovCommand extends Command {
* @return {Array} args for nyc
* @protected
*/
getCovArgs(context) {
* getCovArgs(context) {
let covArgs = [
// '--show-process-tree',
];
Expand All @@ -128,7 +128,7 @@ class CovCommand extends Command {
covArgs.push(exclude);
}
covArgs.push(require.resolve('mocha/bin/_mocha'));
const testArgs = this.formatTestArgs(context);
const testArgs = yield this.formatTestArgs(context);
if (!testArgs) return;
covArgs = covArgs.concat(testArgs);
return covArgs;
Expand Down
44 changes: 40 additions & 4 deletions lib/cmd/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const fs = require('fs');
const path = require('path');
const globby = require('globby');
const Command = require('../command');
const changed = require('jest-changed-files');

class TestCommand extends Command {
constructor(rawArgv) {
Expand All @@ -29,6 +30,10 @@ class TestCommand extends Command {
'full-trace': {
description: 'display the full stack trace',
},
changed: {
description: 'only test with changed files and match ${cwd}/test/**/*.test.(js|ts)',
alias: 'c',
},
};
}

Expand All @@ -44,7 +49,7 @@ class TestCommand extends Command {
execArgv: context.execArgv,
};
const mochaFile = require.resolve('mocha/bin/_mocha');
const testArgs = this.formatTestArgs(context);
const testArgs = yield this.formatTestArgs(context);
if (!testArgs) return;
debug('run test: %s %s', mochaFile, testArgs.join(' '));
yield this.helper.forkNode(mochaFile, testArgs, opt);
Expand All @@ -69,7 +74,7 @@ class TestCommand extends Command {
* @return {Array} [ '--require=xxx', 'xx.test.js' ]
* @protected
*/
formatTestArgs({ argv, debug }) {
* formatTestArgs({ argv, debug }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

要看下内网版会不会有影响

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个 breaking change 了。。。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

内网版本没有依赖这个方法,这个应该是私有的吧?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不一定,得看看具体情况,先在调用的地方加个判断兼容一下吧,如果是 generator function 再 yield。下一个版本全部改为 aa

Copy link
Member

@atian25 atian25 Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

common-bin 的 helper 里面有 callFn 方法

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没看懂,我看过内部的版本,没其他地方调用这个方法了。

在 egg-bin 里面用 callFn 没有意义,因为这个方法已经是 generator function 了。这里的 breaking change 就是 formateTestArgs 变成了 generator function,如果下层的模块依赖了这个方法就会挂。

现在来看不管怎样也不会影响到线上服务,先这样发掉。真出问题了改一下下游的调用代码好了。

const testArgv = Object.assign({}, argv);

/* istanbul ignore next */
Expand Down Expand Up @@ -109,10 +114,27 @@ class TestCommand extends Command {

testArgv.require = requireArr;

let pattern;
// changed
if (testArgv.changed) {
pattern = yield this._getChangedTestFiles();
if (!pattern.length) {
console.log('No changed test files');
return;
}
}

if (!pattern) {
// specific test files
pattern = testArgv._.slice();
}
if (!pattern.length && process.env.TESTS) {
pattern = process.env.TESTS.split(',');
}

// collect test files
let pattern = testArgv._.slice();
if (!pattern.length) {
pattern = [ process.env.TESTS || `test/**/*.test.${testArgv.typescript ? 'ts' : 'js'}` ];
pattern = [ `test/**/*.test.${testArgv.typescript ? 'ts' : 'js'}` ];
}
pattern = pattern.concat([ '!test/fixtures', '!test/node_modules' ]);

Expand Down Expand Up @@ -141,6 +163,20 @@ class TestCommand extends Command {

return this.helper.unparseArgv(testArgv);
}

* _getChangedTestFiles() {
const cwd = process.cwd();
const res = yield changed.getChangedFilesForRoots([ cwd ]);
const changedFiles = res.changedFiles;
const files = [];
for (const file of changedFiles) {
// only find ${cwd}/test/**/*.test.(js|ts)
if (file.startsWith(path.join(cwd, 'test')) && file.match(/\.test\.(js|ts)$/)) {
files.push(file);
}
}
return files;
}
}

module.exports = TestCommand;
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@
"globby": "^8.0.1",
"inspector-proxy": "^1.2.1",
"intelli-espower-loader": "^1.0.1",
"source-map-support": "^0.5.6",
"jest-changed-files": "^23.4.2",
"mocha": "^5.2.0",
"mz-modules": "^2.1.0",
"nyc": "^13.0.1",
"power-assert": "^1.6.0",
"semver": "^5.5.0",
"source-map-support": "^0.5.6",
"test-exclude": "^5.0.0",
"ts-node": "^7.0.0",
"ypkgfiles": "^1.6.0"
Expand Down
42 changes: 42 additions & 0 deletions test/lib/cmd/test.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const coffee = require('coffee');
const mm = require('mm');
const assert = require('assert');
const semver = require('semver');
const changed = require('jest-changed-files');
const Command = require('../../../lib/cmd/test');

describe('test/lib/cmd/test.test.js', () => {
const eggBin = require.resolve('../../../bin/egg-bin.js');
Expand Down Expand Up @@ -45,6 +47,16 @@ describe('test/lib/cmd/test.test.js', () => {
.end(done);
});

it('should only test files specified by TESTS with multi pattern', done => {
mm(process.env, 'TESTS', 'test/a.test.js,test/b/b.test.js');
coffee.fork(eggBin, [ 'test' ], { cwd })
.expect('stdout', /should success/)
.expect('stdout', /a\.test\.js/)
.expect('stdout', /b\/b.test.js/)
.expect('code', 0)
.end(done);
});

it('should only test files specified by TESTS argv', done => {
mm(process.env, 'TESTS', 'test/**/*.test.js');
coffee.fork(eggBin, [ 'test', 'test/a.test.js' ], { cwd })
Expand Down Expand Up @@ -189,4 +201,34 @@ describe('test/lib/cmd/test.test.js', () => {
});
});
});

// changed need to mock getChangedFilesForRoots, so we just test formatTestArgs directly
describe('changed', () => {
it('should return undefined if no test file changed', function* () {
const cmd = new Command([ '--changed' ]);
mm.data(changed, 'getChangedFilesForRoots', {
changedFiles: new Set(),
});
const args = yield cmd.formatTestArgs(cmd.context);
assert(!args);
});

it('should return file changed', function* () {
const cmd = new Command([ '--changed' ]);
mm.data(changed, 'getChangedFilesForRoots', {
changedFiles: new Set([ __filename ]),
});
const args = yield cmd.formatTestArgs(cmd.context);
assert(args.includes('--changed', __filename));
});

it('should filter not test file', function* () {
const cmd = new Command([ '--changed' ]);
mm.data(changed, 'getChangedFilesForRoots', {
changedFiles: new Set([ __filename + '.tmp', 'abc.test.js' ]),
});
const args = yield cmd.formatTestArgs(cmd.context);
assert(!args);
});
});
});