From ca4f78f5e7c608fbe9af37577c62a5b64bb2b45c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=90=96=E7=8C=A9?= Date: Mon, 27 Aug 2018 18:51:41 +0800 Subject: [PATCH] fix: fix source map line number incorrect (#107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 修复跑单测时的堆栈行数及列数错误问题。 ### 原因 espower-typescript 里会在 ts 的 sourcemap 基础上再加上 power-assert 的 sourcemap，因为 power-assert 会对源码做处理，然后会返回一个新的 code + sourcemap 然后 ts-node 里面自己内置了 source-map-support ，然后在 source-map-support 拿 code 的时候命中了 ts-node 的缓存 ( https://github.com/TypeStrong/ts-node/blob/master/src/index.ts#L218 )，拿到的是 ts-node 编译后的源码，而不是上面 espower-typescript 处理返回的... 所以导致 source-map-support 在实际处理 source map 映射的时候，code 是 power-assert 编译后的代码，sourceMap 却是 ts 的 sourceMap....就导致了这个问题。 ### 解决方案 在 egg-bin test 的时候，由 egg-bin 来注册 source-map-support，缓存解析后的代码并且传给 sourceMapSupport。 --- Fix bug that error stack gives incorrect line number and column number in unit testing. ### Reason `espower-typescript` combines the sourceMap of `ts-node` and `power-assert` and return a new sourceMap for supporting `power-assert`. But `source-map-support` receives old sourceMap when handling the code because `ts-node` cache its sourceMap in `retrieveFile` method: https://github.com/TypeStrong/ts-node/blob/master/src/index.ts#L218 ```typescript // Install source map support and read from memory cache. sourceMapSupport.install({ environment: 'node', retrieveFile: function (path) { return memoryCache.outputs[path]; } }); ``` ### Solution Overwriting the `retrieveFile` method of `source-map-support` in `egg-bin test` to return a correct sourceMap for code. https://github.com/power-assert-js/espower-typescript/issues/54 --- .gitignore | 2 + lib/cmd/test.js | 3 + lib/correct-source-map.js | 55 +++++++++++++++++++ package.json | 1 + test/fixtures/example-ts-error-stack/app.ts | 9 +++ .../config/config.default.ts | 7 +++ .../node_modules/egg/index.js | 8 +++ .../node_modules/egg/package.json | 3 + .../example-ts-error-stack/package.json | 6 ++ .../example-ts-error-stack/test/index.test.ts | 16 ++++++ .../example-ts-error-stack/tsconfig.json | 19 +++++++ .../example-ts-pkg/node_modules/egg/index.js | 3 +- .../example-ts/node_modules/egg/index.js | 2 +- .../ts/node_modules/aliyun-egg/index.js | 2 +- test/ts.test.js | 38 +++++++++++++ 15 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 lib/correct-source-map.js create mode 100644 test/fixtures/example-ts-error-stack/app.ts create mode 100644 test/fixtures/example-ts-error-stack/config/config.default.ts create mode 100644 test/fixtures/example-ts-error-stack/node_modules/egg/index.js create mode 100644 test/fixtures/example-ts-error-stack/node_modules/egg/package.json create mode 100644 test/fixtures/example-ts-error-stack/package.json create mode 100644 test/fixtures/example-ts-error-stack/test/index.test.ts create mode 100644 test/fixtures/example-ts-error-stack/tsconfig.json diff --git a/.gitignore b/.gitignore index f22ee7c5..3c329524 100644 --- a/.gitignore +++ b/.gitignore @@ -11,11 +11,13 @@ test/fixtures/ts/node_modules/aliyun-egg/ !test/fixtures/test-files-glob/** !test/fixtures/test-files-stack/node_modules/ !test/fixtures/example/node_modules/ +!test/fixtures/example-ts-error-stack/node_modules/ **/run/*.json .tmp .vscode +.cache *.log package-lock.json .nyc_output diff --git a/lib/cmd/test.js b/lib/cmd/test.js index 88a25df0..5f1cb6a5 100644 --- a/lib/cmd/test.js +++ b/lib/cmd/test.js @@ -105,6 +105,9 @@ class TestCommand extends Command { if (testArgv.typescript) { // remove ts-node in context getter on top. requireArr.push(require.resolve('espower-typescript/guess')); + + // use to correct sourceMap mapping + requireArr.push(path.resolve(__dirname, '../correct-source-map')); } testArgv.require = requireArr; diff --git a/lib/correct-source-map.js b/lib/correct-source-map.js new file mode 100644 index 00000000..53aedd96 --- /dev/null +++ b/lib/correct-source-map.js @@ -0,0 +1,55 @@ +/** + * Only works in test/cov command with `espower-typescript`. + * + * Fix bug that error stack gives incorrect line number and column number in unit testing. + * + * ### Reason + * + * `espower-typescript` combines the sourceMap of `ts-node` and `power-assert` and return a new sourceMap + * for supporting `power-assert`. But `source-map-support` receives old sourceMap when handling the code because + * `ts-node` cache its sourceMap in `retrieveFile` method: https://github.com/TypeStrong/ts-node/blob/master/src/index.ts#L218 + * + * ```typescript + * // Install source map support and read from memory cache. + * sourceMapSupport.install({ + * environment: 'node', + * retrieveFile: function (path) { + * return memoryCache.outputs[path]; + * } + * }); + * ``` + * + * ### Solution + * + * Overwriting the `retrieveFile` method of `source-map-support` in `egg-bin test` to return a correct sourceMap for code. + * + * + * https://github.com/eggjs/egg-bin/pull/107 + * https://github.com/power-assert-js/espower-typescript/issues/54 + * + */ + +'use strict'; + +const sourceMapSupport = require('source-map-support'); +const cacheMap = {}; +const extensions = [ '.ts', '.tsx' ]; + +sourceMapSupport.install({ + environment: 'node', + retrieveFile(path) { + return cacheMap[path]; + }, +}); + +for (const ext of extensions) { + const originalExtension = require.extensions[ext]; + require.extensions[ext] = (module, filePath) => { + const originalCompile = module._compile; + module._compile = function(code, filePath) { + cacheMap[filePath] = code; + return originalCompile.call(this, code, filePath); + }; + return originalExtension(module, filePath); + }; +} diff --git a/package.json b/package.json index 7ac5954f..4f36863a 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "globby": "^8.0.1", "inspector-proxy": "^1.2.1", "intelli-espower-loader": "^1.0.1", + "source-map-support": "^0.5.6", "mocha": "^5.2.0", "mz-modules": "^2.1.0", "nyc": "^12.0.2", diff --git a/test/fixtures/example-ts-error-stack/app.ts b/test/fixtures/example-ts-error-stack/app.ts new file mode 100644 index 00000000..e1a8cc29 --- /dev/null +++ b/test/fixtures/example-ts-error-stack/app.ts @@ -0,0 +1,9 @@ +export default function() { + // placeholder comments + // placeholder comments + // placeholder comments + // placeholder comments + if (process.env.THROW_ERROR === 'true') { + throw new Error('throw error'); + } +} \ No newline at end of file diff --git a/test/fixtures/example-ts-error-stack/config/config.default.ts b/test/fixtures/example-ts-error-stack/config/config.default.ts new file mode 100644 index 00000000..106950bc --- /dev/null +++ b/test/fixtures/example-ts-error-stack/config/config.default.ts @@ -0,0 +1,7 @@ +'use strict'; + +export default () => { + const config = {} as any; + config.keys = '123456'; + return config; +}; diff --git a/test/fixtures/example-ts-error-stack/node_modules/egg/index.js b/test/fixtures/example-ts-error-stack/node_modules/egg/index.js new file mode 100644 index 00000000..7102f7f7 --- /dev/null +++ b/test/fixtures/example-ts-error-stack/node_modules/egg/index.js @@ -0,0 +1,8 @@ +'use strict'; + +module.exports = require('../../../../../node_modules/egg'); + +setTimeout(() => { + console.log('exit by master test end') + process.exit(0); +}, require('os').platform() === 'win32' ? 10000 : 6000); diff --git a/test/fixtures/example-ts-error-stack/node_modules/egg/package.json b/test/fixtures/example-ts-error-stack/node_modules/egg/package.json new file mode 100644 index 00000000..6697ad3f --- /dev/null +++ b/test/fixtures/example-ts-error-stack/node_modules/egg/package.json @@ -0,0 +1,3 @@ +{ + "name": "egg" +} diff --git a/test/fixtures/example-ts-error-stack/package.json b/test/fixtures/example-ts-error-stack/package.json new file mode 100644 index 00000000..fdbe7648 --- /dev/null +++ b/test/fixtures/example-ts-error-stack/package.json @@ -0,0 +1,6 @@ +{ + "name": "example-ts-error-stack", + "egg": { + "typescript": true + } +} \ No newline at end of file diff --git a/test/fixtures/example-ts-error-stack/test/index.test.ts b/test/fixtures/example-ts-error-stack/test/index.test.ts new file mode 100644 index 00000000..d77166d3 --- /dev/null +++ b/test/fixtures/example-ts-error-stack/test/index.test.ts @@ -0,0 +1,16 @@ +'use strict'; + +// import * as assert from 'assert'; + +describe('test/index.test.ts', () => { + // placeholder comments + it('should throw error', async () => { + throw new Error('error'); + }); + + // placeholder comments + it('should assert', async () => { + const obj = { key: '111' }; + assert(obj.key === '222'); + }); +}); diff --git a/test/fixtures/example-ts-error-stack/tsconfig.json b/test/fixtures/example-ts-error-stack/tsconfig.json new file mode 100644 index 00000000..4f10abf7 --- /dev/null +++ b/test/fixtures/example-ts-error-stack/tsconfig.json @@ -0,0 +1,19 @@ +{ + "compilerOptions": { + "target": "es2017", + "module": "commonjs", + "strict": true, + "noImplicitAny": false, + "moduleResolution": "node", + "experimentalDecorators": true, + "emitDecoratorMetadata": true, + "pretty": true, + "noUnusedLocals": true, + "noUnusedParameters": true, + "noFallthroughCasesInSwitch": true, + "skipLibCheck": true, + "skipDefaultLibCheck": true, + "inlineSourceMap": true, + "importHelpers": true + }, +} \ No newline at end of file diff --git a/test/fixtures/example-ts-pkg/node_modules/egg/index.js b/test/fixtures/example-ts-pkg/node_modules/egg/index.js index d1757bf0..b10b2d46 100644 --- a/test/fixtures/example-ts-pkg/node_modules/egg/index.js +++ b/test/fixtures/example-ts-pkg/node_modules/egg/index.js @@ -5,4 +5,5 @@ module.exports = require('../../../../../node_modules/egg'); setTimeout(() => { console.log('exit by master test end') process.exit(0); -}, 6000); +}, require('os').platform() === 'win32' ? 10000 : 6000); + diff --git a/test/fixtures/example-ts/node_modules/egg/index.js b/test/fixtures/example-ts/node_modules/egg/index.js index d1757bf0..7102f7f7 100644 --- a/test/fixtures/example-ts/node_modules/egg/index.js +++ b/test/fixtures/example-ts/node_modules/egg/index.js @@ -5,4 +5,4 @@ module.exports = require('../../../../../node_modules/egg'); setTimeout(() => { console.log('exit by master test end') process.exit(0); -}, 6000); +}, require('os').platform() === 'win32' ? 10000 : 6000); diff --git a/test/fixtures/ts/node_modules/aliyun-egg/index.js b/test/fixtures/ts/node_modules/aliyun-egg/index.js index c1500965..1d072436 100644 --- a/test/fixtures/ts/node_modules/aliyun-egg/index.js +++ b/test/fixtures/ts/node_modules/aliyun-egg/index.js @@ -15,6 +15,6 @@ module.exports.startCluster = options => { setTimeout(() => { console.log('exit by master test end') process.exit(0); - }, 5000); + }, require('os').platform() === 'win32' ? 10000 : 6000); return egg.startCluster(options); }; diff --git a/test/ts.test.js b/test/ts.test.js index e914bd80..12f24993 100644 --- a/test/ts.test.js +++ b/test/ts.test.js @@ -74,6 +74,44 @@ describe('test/ts.test.js', () => { }); }); + describe('error stacks', () => { + if (process.env.EGG_VERSION && process.env.EGG_VERSION === '1') { + console.log('skip egg@1'); + return; + } + + before(() => { + cwd = path.join(__dirname, './fixtures/example-ts-error-stack'); + }); + + it('should correct error stack line number in starting app', () => { + mm(process.env, 'THROW_ERROR', 'true'); + return coffee.fork(eggBin, [ 'dev' ], { cwd }) + // .debug() + .expect('stderr', /Error: throw error/) + .expect('stderr', /at \w+ \(.+app\.ts:7:11\)/) + .end(); + }); + + it('should correct error stack line number in testing app', () => { + return coffee.fork(eggBin, [ 'test' ], { cwd }) + // .debug() + .expect('stdout', /error/) + .expect('stdout', /at Context\.it .+index\.test\.ts:8:11\)/) + .expect('stdout', /at Context\.it .+index\.test\.ts:14:5\)/) + .end(); + }); + + it('should correct error stack line number in covering app', () => { + return coffee.fork(eggBin, [ 'test' ], { cwd }) + // .debug() + .expect('stdout', /error/) + .expect('stdout', /at Context\.it .+index\.test\.ts:8:11\)/) + .expect('stdout', /at Context\.it .+index\.test\.ts:14:5\)/) + .end(); + }); + }); + describe('egg.typescript = true', () => { if (process.env.EGG_VERSION && process.env.EGG_VERSION === '1') { console.log('skip egg@1');