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: simplify mocha error stack #59

Merged
merged 1 commit into from
Jun 8, 2017
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 .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ coverage/
!test/fixtures/custom-framework-app/node_modules/
!test/fixtures/demo-app/node_modules/aliyun-egg/node_modules/
!test/fixtures/test-files-glob/**
!test/fixtures/test-files-stack/node_modules/
.tmp
.vscode
*.log
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ You can pass any mocha argv.
- `--require` require the given module
- `--grep` only run tests matching <pattern>
- `--timeout` milliseconds, default to 30000
- `--full-trace` display the full stack trace, default to false.
Copy link
Member Author

Choose a reason for hiding this comment

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

加个一个参数来显示全部,这个参数也是 mocha 自己的参数,会透传过去。(不过传递给 mocha 好像没啥区别)

Copy link
Member

Choose a reason for hiding this comment

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

mochajs/mocha#545 感觉是在做同一件事情

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 Author

@atian25 atian25 Jun 7, 2017

Choose a reason for hiding this comment

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

https://github.com/mochajs/mocha/blob/master/lib/utils.js#L738

发现 mocha 里面实际已经有了 mocha-clean 的代码的,但它的匹配方式不兼容我们的 npminstall 格式。

我们这边先自己实现吧,回头我去提个 PR 到 mocha。(我们的一些规则没办法提到 PR 那边的,如 co-mocha, power-assert,还是直接在这边实现好了。)

cc @fengmk2

- see more at https://mochajs.org/#usage

#### environment
Expand Down
7 changes: 7 additions & 0 deletions lib/cmd/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ class TestCommand extends Command {
alias: 't',
type: 'number',
},
'full-trace': {
description: 'display the full stack trace',
},
};
}

Expand Down Expand Up @@ -69,6 +72,10 @@ class TestCommand extends Command {
/* istanbul ignore next */
if (!Array.isArray(requireArr)) requireArr = [ requireArr ];

// clean mocha stack, inspired by https://github.com/rstacruz/mocha-clean
// [mocha built-in](https://github.com/mochajs/mocha/blob/master/lib/utils.js#L738) don't work with `[npminstall](https://github.com/cnpm/npminstall)`, so we will override it.
if (!testArgv.fullTrace) requireArr.unshift(require.resolve('../mocha-clean'));

requireArr.push(require.resolve('co-mocha'));

if (requireArr.includes('intelli-espower-loader')) {
Expand Down
39 changes: 39 additions & 0 deletions lib/mocha-clean.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use strict';

const mocha = require('mocha');
const internal = [
Copy link
Member Author

@atian25 atian25 Jun 7, 2017

Choose a reason for hiding this comment

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

这里的规则要仔细看看。
6, 7 ,8 的堆栈输出有些不同,尤其是异步调用,调了好久。

'(timers.js:',
'(node.js:',
'(module.js:',
'(domain.js:',
'GeneratorFunctionPrototype.next (native)',
'at Generator.next',
// node 8.x
'at Promise (<anonymous>)',
'at next (native)',
'__mocha_internal__',
/node_modules\/.*empower-core\//,
/node_modules\/.*mocha\//,
/node_modules\/.*co\//,
/node_modules\/.*co-mocha\//,
];

// monkey-patch `Runner#fail` to modify stack
const originFn = mocha.Runner.prototype.fail;
mocha.Runner.prototype.fail = function(test, err) {
/* istanbul ignore else */
if (err.stack) {
const stack = err.stack.split('\n').filter(line => {
line = line.replace(/\\\\?/g, '/');
return !internal.some(rule => match(line, rule));
});
stack.push(' [use `--full-trace` to display the full stack trace]');
err.stack = stack.join('\n');
}
return originFn.call(this, test, err);
};

function match(line, rule) {
if (rule instanceof RegExp) return rule.test(line);
return line.includes(rule);
}
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@
"co-mocha": "^1.2.0",
"common-bin": "^2.4.0",
"debug": "^2.6.8",
"detect-port": "^1.1.3",
"detect-port": "^1.2.0",
"egg-utils": "^2.2.0",
"globby": "^6.1.0",
"intelli-espower-loader": "^1.0.1",
"istanbul": "^1.1.0-alpha.1",
"mocha": "^3.4.2",
"mz-modules": "^1.0.0",
"power-assert": "^1.4.2",
"power-assert": "^1.4.3",
"ypkgfiles": "^1.4.0"
},
"devDependencies": {
Expand Down
7 changes: 7 additions & 0 deletions test/fixtures/test-files-stack/node_modules/my-sleep/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions test/fixtures/test-files-stack/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "test-files-stack"
}
11 changes: 11 additions & 0 deletions test/fixtures/test-files-stack/test/assert.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';

const assert = require('assert');

describe('assert.test.js', () => {
it('should fail with simplify stack', () => {
[ 1 ].forEach(() => {
assert(1 === 0);
});
});
});
13 changes: 13 additions & 0 deletions test/fixtures/test-files-stack/test/promise.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';

const co = require('co');

describe('promise.test.js', () => {
it('should fail with simplify stack', function* () {
yield co(function* () {
return yield new Promise((resolve, reject) => {
reject(new Error('this is an error'));
});
});
});
});
11 changes: 11 additions & 0 deletions test/fixtures/test-files-stack/test/sleep.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';

const sleep = require('my-sleep');

describe('sleep.test.js', () => {
it('should fail with simplify stack', done => {
sleep(() => {
done(new Error('this is an error'));
});
});
});
2 changes: 1 addition & 1 deletion test/lib/cmd/cov.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ describe('test/lib/cmd/cov.test.js', () => {
mm(process.env, 'TESTS', 'noexist.js');
const cwd = path.join(__dirname, '../../fixtures/prerequire');
yield coffee.fork(eggBin, [ 'cov' ], { cwd })
.debug()
// .debug()
.coverage(false)
.expect('code', 0)
.end();
Expand Down
60 changes: 60 additions & 0 deletions test/lib/cmd/test.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const path = require('path');
const coffee = require('coffee');
const mm = require('mm');
const assert = require('assert');

describe('test/lib/cmd/test.test.js', () => {
const eggBin = require.resolve('../../../bin/egg-bin.js');
Expand Down Expand Up @@ -107,4 +108,63 @@ describe('test/lib/cmd/test.test.js', () => {
.expect('code', 0)
.end();
});

describe('simplify mocha error stack', () => {
const cwd = path.join(__dirname, '../../fixtures/test-files-stack');

it('should clean assert error stack', done => {
mm(process.env, 'TESTS', 'test/assert.test.js');
coffee.fork(eggBin, [ 'test' ], { cwd })
// .debug()
.end((err, { stdout, code }) => {
assert(stdout.match(/AssertionError/));
assert(stdout.match(/at forEach .*assert.test.js:\d+:\d+/));
assert(stdout.match(/at Context.it .*assert.test.js:\d+:\d+/));
assert(stdout.match(/\bat\s+/g).length === 3);
assert(code === 1);
done(err);
});
});

it('should should show full stack trace', done => {
mm(process.env, 'TESTS', 'test/assert.test.js');
coffee.fork(eggBin, [ 'test', '--full-trace' ], { cwd })
// .debug()
.end((err, { stdout, code }) => {
assert(stdout.match(/AssertionError/));
assert(stdout.match(/at .*node_modules.*mocha/));
assert(stdout.match(/\bat\s+/g).length > 10);
assert(code === 1);
done(err);
});
});

it('should clean co error stack', done => {
mm(process.env, 'TESTS', 'test/promise.test.js');
coffee.fork(eggBin, [ 'test' ], { cwd })
// .debug()
.end((err, { stdout, code }) => {
assert(stdout.match(/Error: this is an error/));
assert(stdout.match(/at Promise .*promise.test.js:\d+:\d+/));
assert(stdout.match(/at Context\.<anonymous> .*promise.test.js:\d+:\d+/));
assert(stdout.match(/\bat\s+/g).length === 3);
assert(code === 1);
done(err);
});
});

it('should clean callback error stack', done => {
mm(process.env, 'TESTS', 'test/sleep.test.js');
coffee.fork(eggBin, [ 'test' ], { cwd })
// .debug()
.end((err, { stdout, code }) => {
assert(stdout.match(/Error: this is an error/));
assert(stdout.match(/at sleep .*sleep.test.js:\d+:\d+/));
assert(stdout.match(/at Timeout.setTimeout .*node_modules.*my-sleep.*index.js:\d+:\d+/));
assert(stdout.match(/\bat\s+/g).length === 2);
assert(code === 1);
done(err);
});
});
});
});