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

feat: simplify mocha error stack #59

merged 1 commit into from
Jun 8, 2017

Conversation

atian25
Copy link
Member

@atian25 atian25 commented Jun 7, 2017

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

close eggjs/egg#1010

image

@mention-bot
Copy link

@atian25, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fengmk2, @dead-horse and @popomore to be potential reviewers.

@codecov
Copy link

codecov bot commented Jun 7, 2017

Codecov Report

Merging #59 into master will increase coverage by 1.67%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
+ Coverage   77.27%   78.94%   +1.67%     
==========================================
  Files           8        9       +1     
  Lines         176      190      +14     
==========================================
+ Hits          136      150      +14     
  Misses         40       40
Impacted Files Coverage Δ
lib/mocha-clean.js 100% <100%> (ø)
lib/cmd/test.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77d7e6d...2ca351d. Read the comment docs.

@atian25 atian25 force-pushed the mocha-clean-stack branch 3 times, most recently from 5332fba to 981dfe2 Compare June 7, 2017 06:35
@dead-horse
Copy link
Member

如果用了 generatorFunction 是不是会有 co 的堆栈,也干掉?

@atian25 atian25 force-pushed the mocha-clean-stack branch 2 times, most recently from 6d56944 to 3ad7d0f Compare June 7, 2017 09:17
@atian25
Copy link
Member Author

atian25 commented Jun 7, 2017

@dead-horse 如下

describe('clean.js', () => {
  it('should fail with simplify stack', function* () {
    yield co(function* test(){
      [ 1 ].forEach(() => {
        throw new Error('clean.js throw');
      });
    });
  });
});

最终效果:
image

干掉的内容如下 diff:

  • co 是命中了 /node_modules\/.*co\// 这个规则,被干掉了
  • at next (native) 手动干掉
  • at Array.forEach (native) 这个不干掉
  clean.js
Error: clean.js throw
    at forEach (/Users/tz/Workspaces/eggjs/egg-bin/test/fixtures/test-files/test/clean.js:9:15)
    at Array.forEach (native)
    at test (/Users/tz/Workspaces/eggjs/egg-bin/test/fixtures/test-files/test/clean.js:8:13)
-     at next (native)
-     at onFulfilled (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@co/index.js:65:19)
-     at /Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@co/index.js:54:5
-     at co (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@co/index.js:50:10)
    at Context.<anonymous> (/Users/tz/Workspaces/eggjs/egg-bin/test/fixtures/test-files/test/clean.js:7:11)
-     at next (native)
-     at onFulfilled (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@co/index.js:65:19)
-     at /Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@co/index.js:54:5
-     at Context.co (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@co/index.js:50:10)
-     at Context.createPromise (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@co/index.js:30:15)
-     at callFn (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@mocha/lib/runnable.js:348:21)
-     at Test.Runnable.run (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@mocha/lib/runnable.js:340:7)
-     at Test.Runnable.run (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@co-mocha/lib/co-mocha.js:43:16)
-     at Runner.runTest (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@mocha/lib/runner.js:443:10)
-     at /Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@mocha/lib/runner.js:549:12
-     at next (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@mocha/lib/runner.js:361:14)
-     at /Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@mocha/lib/runner.js:371:7
-     at next (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@mocha/lib/runner.js:295:14)
-     at Immediate.<anonymous> (/Users/tz/Workspaces/eggjs/egg-bin/node_modules/[email protected]@mocha/lib/runner.js:339:5)
-     at runCallback (timers.js:651:20)
-     at tryOnImmediate (timers.js:624:5)
-     at processImmediate [as _immediateCallback] (timers.js:596:5)

@dead-horse
Copy link
Member

node_modules 建议不要干掉,有时候确实是底层的依赖报错了,这样堆栈就没了,还是将一些确定没用的干掉就好

@atian25
Copy link
Member Author

atian25 commented Jun 7, 2017

那就干掉 mocha, power-assert, co, co-mocha ?

要加一个 --verbose--fullstack 么?

@atian25 atian25 force-pushed the mocha-clean-stack branch 4 times, most recently from ba2371e to 7e05c58 Compare June 7, 2017 13:23
@popomore
Copy link
Member

popomore commented Jun 7, 2017

是不是把 stack 长度加长

@atian25 atian25 force-pushed the mocha-clean-stack branch from 7e05c58 to 2455e76 Compare June 7, 2017 14:39
@atian25
Copy link
Member Author

atian25 commented Jun 7, 2017

@atian25 atian25 force-pushed the mocha-clean-stack branch 3 times, most recently from e457a4c to 45c5e77 Compare June 7, 2017 15:57
'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 的堆栈输出有些不同,尤其是异步调用,调了好久。

@@ -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

if (err.stack) {
const stack = err.stack.split('\n').filter(line => {
line = line.replace(/\\\\?/g, '/');
// console.log('[%s] %s', !internal.some(rule => match(line, rule)) ? ' ' : '--', line);
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 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

Choose a reason for hiding this comment

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

去掉了

@atian25 atian25 force-pushed the mocha-clean-stack branch from 45c5e77 to 5adc517 Compare June 8, 2017 00:11
@atian25 atian25 force-pushed the mocha-clean-stack branch from 5adc517 to 2ca351d Compare June 8, 2017 00:27
Copy link
Member

@fengmk2 fengmk2 left a comment

Choose a reason for hiding this comment

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

测试体验又提升了

@atian25 atian25 merged commit 0b01158 into master Jun 8, 2017
@atian25 atian25 deleted the mocha-clean-stack branch June 8, 2017 02:36
@atian25
Copy link
Member Author

atian25 commented Jun 8, 2017

3.5.0

fengmk2 pushed a commit that referenced this pull request Dec 17, 2022
[skip ci]

## 1.0.0 (2022-12-17)

### Features

* add `egg-bin autod --check` command ([#70](#70)) ([0f0c7ab](0f0c7ab))
* add COV_EXCLUDES for coverage excludes ([#7](#7)) ([a33e561](a33e561))
* add sticky mode support ([#32](#32)) ([f5a1152](f5a1152))
* add ts env in command ([#98](#98)) ([756310e](756310e))
* allow loading ts compiler from cwd ([#169](#169)) ([4a54cec](4a54cec))
* auto detect available port ([#22](#22)) ([6d7ad41](6d7ad41))
* auto require setup file ([#24](#24)) ([a847750](a847750))
* build-in intelli-espower-loader ([#20](#20)) ([4e86a67](4e86a67))
* change default timeout to 60000 ([#50](#50)) ([cbc9263](cbc9263))
* commands support specific execArgv(harmony) ([#33](#33)) ([0c7108b](0c7108b))
* cov support output json-summary ([#64](#64)) ([c1de734](c1de734))
* cov support typescript ([#91](#91)) ([8a05e19](8a05e19))
* **cov:** add nyc instrument passthrough ([#103](#103)) ([8fa805b](8fa805b))
* **cov:** add prerequire option ([#53](#53)) ([055c47f](055c47f))
* **cov:** add prerequire option ([#53](#53)) ([a6a2b4a](a6a2b4a))
* debug proxy support ([678b83d](678b83d))
* **debug:** [BREAKING_CHANGE] remove iron-node ([#26](#26)) ([9d4170f](9d4170f))
* default enable c8 report ([#189](#189)) ([72e925b](72e925b))
* **dev:** pass debug args to execArgv ([#12](#12)) ([92b111f](92b111f))
* egg-bin check ([#87](#87)) ([92b1489](92b1489))
* enable mochawesome by default ([#193](#193)) ([6636e8f](6636e8f))
* expose proc ([#152](#152)) ([dcc9b25](dcc9b25))
* extractArgv refactor & extract debug port ([994616d](994616d))
* extractArgv support expose_debug_as ([6f5d525](6f5d525))
* impl parallel for mocha parallel mode ([#185](#185)) ([78141e8](78141e8))
* intergration with egg-ts-helper ([#123](#123)) ([263cfd1](263cfd1))
* pass --check to pkgfiles ([#48](#48)) ([7b0c995](7b0c995))
* remove correct-source-map.js ([#109](#109)) ([8b62742](8b62742))
* resolve istanbul path for coffee ([#9](#9)) ([147add8](147add8))
* revert egg-bin check ([#90](#90)) ([83322e6](83322e6))
* revert to 4.2.0 ([c65a00d](c65a00d))
* set EGG_MASTER_CLOSE_TIMEOUT when run dev ([#88](#88)) ([c48860c](c48860c))
* should print error stack ([#132](#132)) ([5c621f6](5c621f6))
* simplify mocha error stack ([#59](#59)) ([0b01158](0b01158))
* support $NODE_DEBUG_OPTION ([#71](#71)) ([1340ce7](1340ce7))
* support c8 report ([#172](#172)) ([1e96da2](1e96da2))
* support cov command  in win32 ([#52](#52)) ([93b731d](93b731d))
* support egg-bin test --changed ([#111](#111)) ([51f93aa](51f93aa))
* support egg.typescript ([#92](#92)) ([a7f0ca8](a7f0ca8))
* support mocha custom require args ([#5](#5)) ([fb8fd32](fb8fd32))
* support read egg.require from package.json ([#121](#121)) ([904103f](904103f))
* support set eggTsHelper ([#183](#183)) ([f564cbf](f564cbf))
* support switch ts compiler ([#158](#158)) ([a74bae2](a74bae2))
* support typescript ([#89](#89)) ([75b5cd6](75b5cd6))
* test  --dry-run ([#145](#145)) ([3cc3b0b](3cc3b0b))
* try to use --inspect first ([#19](#19)) ([d7ad24c](d7ad24c))
* update pkg.files that if file exists ([#37](#37)) ([af5af6a](af5af6a))
* upgrade espower-typescript to 9.0 ([#106](#106)) ([35e89db](35e89db))
* use nyc instead of istanbul ([#63](#63)) ([3cf312c](3cf312c))
* use test when run cov on Windows ([#18](#18)) ([611027f](611027f)), closes [/github.com/eggjs/egg/pull/133#issuecomment-256827488](https://github.com/eggjs//github.com/eggjs/egg/pull/133/issues/issuecomment-256827488)
* use unparseArgv from common-bin ([#45](#45)) ([da41b8e](da41b8e))

### Bug Fixes

* --full-trace should be boolean ([#191](#191)) ([bfd7fab](bfd7fab))
* -x only support string ([#47](#47)) ([b79f0cc](b79f0cc))
* .setup.js should be the first test file ([#30](#30)) ([4f45c6b](4f45c6b))
* add missing deps ([#42](#42)) ([86ebc75](86ebc75))
* add power-assert to deps ([#21](#21)) ([8b7ce8d](8b7ce8d))
* auto add .setup.ts file ([#147](#147)) ([00afdf7](00afdf7))
* can not find iron-node in subprocess ([#8](#8)) ([d6a57f5](d6a57f5))
* ci failed ([#162](#162)) ([4a076e6](4a076e6))
* clean more mocha error stack ([#60](#60)) ([9c77118](9c77118))
* conflix source map support ([#181](#181)) ([a1ec4f7](a1ec4f7))
* cov replaced warning at win ([#49](#49)) ([d2850a5](d2850a5))
* **cov:** istanbul path env ([#44](#44)) ([ce8f141](ce8f141))
* **cov:** wait 1 second for Windows ([#16](#16)) ([742f6fc](742f6fc))
* debug at 6.x ([469739f](469739f))
* debug mode detect ([#130](#130)) ([819d78f](819d78f))
* debug-test invoke formatTestArgs ([a82a87a](a82a87a))
* don't pass prerequire ([#57](#57)) ([e0d03d3](e0d03d3))
* downgrade ts-node ([#126](#126)) ([d802694](d802694))
* egginfo is not exists ([#159](#159)) ([8666e9e](8666e9e))
* ets not found ([#124](#124)) ([8f6135e](8f6135e))
* fix cov env ([#188](#188)) ([e18ceda](e18ceda))
* fix ENABLE_MOCHA_PARALLEL/AUTO_AGENT env ([#187](#187)) ([88ba6d5](88ba6d5))
* fix espwoer-typescript inject logic ([#178](#178)) ([2e0fecd](2e0fecd))
* fix source map line number incorrect ([#107](#107)) ([ca4f78f](ca4f78f)), closes [/github.com/TypeStrong/ts-node/blob/master/src/index.ts#L218](https://github.com/eggjs//github.com/TypeStrong/ts-node/blob/master/src/index.ts/issues/L218)
* fixed ts-node ignore files ([#105](#105)) ([a790304](a790304))
* force exit when runner complete ([#83](#83)) ([7386194](7386194))
* ignore eggTsHelper on node-test ([#186](#186)) ([c5db00e](c5db00e))
* ignore frontend files and document files ([#65](#65)) ([f9d578e](f9d578e))
* let sub class can override getFrameworkOrEggPath ([1320144](1320144))
* link mocha bin from inner file ([#15](#15)) ([f6cb195](f6cb195))
* make sure dev command eggPath can be override ([#23](#23)) ([22510d7](22510d7))
* make sure files sort in all platforms ([#86](#86)) ([6689962](6689962))
* nyc shim ([#138](#138)) ([3b370ef](3b370ef))
* only hotfix spawn-wrap on windows ([#69](#69)) ([e784a3d](e784a3d))
* package.json to reduce vulnerabilities ([#108](#108)) ([e910ae3](e910ae3))
* package.json to reduce vulnerabilities ([#81](#81)) ([e3c33e9](e3c33e9))
* remove espower typescript ([#160](#160)) ([563923a](563923a))
* remove temp excludes ([40aaca1](40aaca1))
* remove ts extensions by default ([#94](#94)) ([1c860a9](1c860a9))
* revert nyc ([#140](#140)) ([9cb8125](9cb8125)), closes [/github.com//pull/133#issuecomment-489387659](https://github.com/eggjs//github.com/eggjs/egg-bin/pull/133/issues/issuecomment-489387659)
* should exit when no test files found ([#100](#100)) ([e375ba4](e375ba4))
* should ignore fixtures and node_modules ([#96](#96)) ([e73c569](e73c569))
* should not timeout when debugging ([#129](#129)) ([3b6819c](3b6819c))
* should only read pkg if argv.typescript not pass ([#97](#97)) ([1e93020](1e93020))
* should pass customEgg to startCluster ([#25](#25)) ([e3d7974](e3d7974))
* should support -p ([#27](#27)) ([df86893](df86893))
* should support multi exclude dirs ([#66](#66)) ([1ac9d68](1ac9d68))
* support --workers same as egg-scripts ([#127](#127)) ([fcae123](fcae123))
* support egg-bin dev --cluster and --baseDir ([#36](#36)) ([ba7d409](ba7d409))
* support node4 ([#35](#35)) ([dbbaf98](dbbaf98))
* support relative path ([#95](#95)) ([7531faa](7531faa))
* There is a case sensitive issue from spawn-wrap  on Windows ([#67](#67)) ([56f8518](56f8518))
* use co-mocha instead of thunk-mocha ([#38](#38)) ([f6b5171](f6b5171))
* use context.env instead of process.env ([#58](#58)) ([9aa0030](9aa0030))
* use inspector at 7.x+ ([#74](#74)) ([2e3498e](2e3498e))
* using ts-node in app should check tscompiler and deps ([#170](#170)) ([662b9e9](662b9e9))
* wait more time for Window 😢 ([#17](#17)) ([d6f6b3b](d6f6b3b))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

enable mocha-clean in egg-bin test
5 participants