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

Typescript support? #12

Closed
el-davo opened this issue May 16, 2017 · 10 comments
Closed

Typescript support? #12

el-davo opened this issue May 16, 2017 · 10 comments

Comments

@el-davo
Copy link

el-davo commented May 16, 2017

Hi, I'm just wondering does this work with typescript. I have tried and couldn't seem to get the watcher working with .ts or .tsx files. output is this, basically it doesn't seem to count my files as being changed and the coverage doesn't seem to be output. When i use nyc and mocha directly I can get the coverage for typescript and the watch works

image

This is my scripts

{
     "scripts": {
      "testit": "cross-env NODE_ENV=test mochista --test-files ./app/**/*.{ts,tsx} ./app/**/*.spec.{ts,tsx} -
          compilers ts:ts-node/register --require should --require ./test-setup.ts",
         "testit:watch": "npm run testit -- -w --all -vvv"
     }
}

Awesome module btw 💃

@laggingreflex
Copy link
Owner

laggingreflex commented May 28, 2017

Try specifying something like --source-files=*.ts because by default I think it'll only consider js,coffee files to be source files. Using -vvv (as you already are) it should display what source files it added. Something like this:

  mochista:log   Globing files... 
  mochista:verb  Globing test files... 
  mochista:verb  Globing source files... 
  mochista:verb  Test files found: 1 
  mochista:silly  src/client/app/components/Breadcrumb/test.js 
>>mochista:verb  Source files found: 82 
  mochista:silly  webpack.config.babel.js 
  mochista:silly  index.js 
  mochista:silly  config.js 
  mochista:silly  src/server/utils/ssl.js 
  mochista:silly  src/server/utils/log.js 
   ...
  mochista:log   Readying watcher... 
>>mochista:verb  Include 3 files 
  mochista:silly  src/**/Breadcrumb/test.js 
  mochista:silly  {src,lib}/**/*.{js,coffee} 
  mochista:silly  *.{js,coffee} 
  mochista:verb  Exclude 3 files 
  mochista:silly  node_modules/** 
  mochista:silly  coverage/** 
  mochista:silly  **/node_modules/** 
>>mochista:silly Watcher event: add config.js 
  mochista:silly Watcher event: add index.js 
  mochista:silly Watcher event: add webpack.config.babel.js 
  mochista:silly Watcher event: addDir src\server 
  mochista:silly Watcher event: addDir src\client 
  mochista:silly Watcher event: addDir src\server\auth 
  ...
>>mochista:silly Watched paths: [object Object]
  mochista:silly ___Inspected object #1___
  mochista:silly { '.': [ 'config.js', 'index.js', 'src', 'webpack.config.babel.js' ],
  mochista:silly   src: [ 'client', 'server' ],
  mochista:silly   'src\**\Breadcrumb\test.js': [],
  mochista:silly   '..': [ 'metamorphic-2017-koa-preact' ],
  mochista:silly   '{src,lib}\**\*.{js,coffee}': [],
  mochista:silly   '*.{js,coffee}': [],
  mochista:silly   'src\client\app\store\models\Profile': [ 'index.js' ],
  mochista:silly   'src\client\app\layout\components\Footer': [ 'index.js', 'style.styl' ],
  ...
  mochista:log   Loading compilers... 

Could you confirm whether you're seeing your source files being added like above?

@laggingreflex
Copy link
Owner

Also please checkout v0.12.0, before which the --all switch wasn't actually the nyc's --all (my bad), it is now.

@el-davo
Copy link
Author

el-davo commented May 28, 2017

Hi @laggingreflex I tried again with the 0.12.1 but i seem to be getting a strange error which wasn't happening before. Has some setting changed?

mochista:log   Globing files... +0ms
  mochista:log   Readying watcher... +31ms
  mochista:warn  Timed out waiting for watcher "ready" event. Proceeding anyway... (See #chokidar issue in the README) +3s
  mochista:log   Loading requires... +9ms
  mochista:error AssertionError: path must be a string
  mochista:error ___Stack trace #1___
  mochista:error AssertionError: path must be a string
  mochista:error     at Module.require (module.js:496:3)
  mochista:error     at requireNative (D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\src\utils\require.js:26:19)
  mochista:error     at tryRequire (D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\src\utils\require.js:12:15)
  mochista:error     at requires.forEach.r (D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\src\mocha\load.js:25:25)
  mochista:error     at Array.forEach (native)
  mochista:error     at loadRequires (D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\src\mocha\load.js:25:12)
  mochista:error     at initLoad (D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\src\mocha\load.js:10:3)
  mochista:error     at Object.<anonymous> (D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\src\mocha\index.js:12:22)
  mochista:error     at next (native)
mochista:error     at step (D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\dist\mocha\index.js:25:191)
  mochista:error     at D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\dist\mocha\index.js:25:437
  mochista:error     at Object.<anonymous> (D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\dist\mocha\index.js:25:99)
  mochista:error     at Object.load (D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\src\mocha\index.js:12:5)
  mochista:error     at D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\src\index.js:22:15
  mochista:error     at next (native)
  mochista:error     at step (D:\git\webpack-react-typescript-boilerplate\node_modules\mochista\dist\index.js:35:191) +37ms

@laggingreflex
Copy link
Owner

Fixed now a3d5c66. Please try again v0.12.2.

I did do a major config-parsing refactor recently fe92749 to make it play better with start-mochista, not sure if this is the only bug. I may have to revert it if anything else pops up. Please let me know.

@el-davo
Copy link
Author

el-davo commented May 29, 2017

Hi @laggingreflex I can confirm that the watcher is now working for typescript with version 0.12.2. However i still cant get any coverage for the typescript. I always get "Unknown" like in the image below

image

My scripts can be seen here - https://github.com/el-davo/webpack-react-typescript-boilerplate/blob/feature/mochista/package.json#L16

Am i right in saying this library ignores the nyc settings in package.json? as can be seen here - https://github.com/el-davo/webpack-react-typescript-boilerplate/blob/feature/mochista/package.json#L96

@el-davo
Copy link
Author

el-davo commented May 29, 2017

I did some fiddling around with it and added .ts and .tsx to this array - https://github.com/laggingreflex/mochista/blob/master/src/istanbul/instrument/index.js#L13 and i get a better looking output now

image

A few observations on this however.

  • .tsx files dont seem to show up
  • When I remove tests it doesn't seem to affect the the coverage, i should see the coverage drop. Interestingly when i reverse this flow, for example i remove the tests and then start the script it shows the reduced coverage and then i re-add in the tests it correctly shows the increased coverage, I then remove the tests again but the coverage does not go down.

@laggingreflex
Copy link
Owner

It doesn't parse nyc settings from package.json. I've implemented it in this pr: #14 (feel free to pull it and test out) but I'm not sure about it (see #13 (comment)).

Use --source-files and --test-files. I personally like using mocha.opts file. In your setup I would use

--compilers ts:ts-node/register
--require should ./test-setup.ts
--source-files app/**/*.ts app/**/*.tsx
--test-files ./app/**/*.spec.ts*

Test files will automatically be excluded from coverage reports, so you don't need the nyc equivalent of "exclude".

@laggingreflex
Copy link
Owner

When I remove tests it doesn't seem to affect the the coverage

By default when you remove tests from a test file only that test file will be triggered as "changed". The source file (for which that test file was written) would still has the same instrumentation cached. (similar issue explain in docs) Use the --run-all switch which will trigger all files to be run (tests to be run, and source to be instrumented) on any file change.

@laggingreflex
Copy link
Owner

Has this been resolved?

I did some fiddling around with it and added .ts and .tsx to this array - https://github.com/laggingreflex/mochista/blob/master/src/istanbul/instrument/index.js#L13

Did you try adding those files via the config (mocha.opts/command-line)?

@laggingreflex
Copy link
Owner

I've completely re-written this.

I've tested it with this forked project and it seems to be working well with typescript: https://github.com/laggingreflex/typescript-react-library

It's now no longer a drop-in replacement for Mocha, so it'll need some configuring to make it work, but it should be pretty easy. Just use --require (--compiler isn't supported, in fact Mocha might drop it as well in future), and you have to specify --extensions .js .ts .tsx

I'm considering this solved but please feel free to reopen if you find issues or have any questions.

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

No branches or pull requests

2 participants