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

Make nyc works with mock-fs #205

Closed
thangngoc89 opened this issue Mar 26, 2016 · 15 comments
Closed

Make nyc works with mock-fs #205

thangngoc89 opened this issue Mar 26, 2016 · 15 comments

Comments

@thangngoc89
Copy link

I'm using mock-fs in my tests for mocking fs.

All tests passed when not using nyc. But when I run coverage with nyc. Both nyc and test fail. The process is killed.

Here is an example:

// test file
import test from "ava"
import mock from "mock-fs"
import compiler from "../compile-template"

test.afterEach(() => {
  mock.restore()
})

test("compile template with lodash.template", async (t) => {
  mock({
    "/some/path": "foo <%= bar %>",
  })

  const result = await compiler("/some/path", {
    bar: "bar",
  })

  t.is(result, "foo bar")
})
$ nyc ava src/_utils/service-worker/__tests__/compile-template.js

  ✖ compile template with lodash.template failed with "ENOENT: no such file or directory, open '/some/path'"
Uncaught Exception: src/_utils/service-worker/__tests__/compile-template.js
  Error: ENOENT, no such file or directory '/home/khoa/web/statinamic/.nyc_output/28496.json'
    Binding.<anonymous> (/home/khoa/web/statinamic/node_modules/mock-fs/lib/binding.js:287:15)
    maybeCallback (/home/khoa/web/statinamic/node_modules/mock-fs/lib/binding.js:41:17)
    Binding.open (/home/khoa/web/statinamic/node_modules/mock-fs/lib/binding.js:274:10)
    Object.fs.openSync (/home/khoa/web/statinamic/node_modules/mock-fs/node/fs-5.0.0.js:584:18)
    Object.fs.writeFileSync (/home/khoa/web/statinamic/node_modules/mock-fs/node/fs-5.0.0.js:1224:33)
    NYC.writeCoverageFile (/home/khoa/web/statinamic/node_modules/nyc/index.js:323:6)
    EventEmitter.onExit.alwaysLast (/home/khoa/web/statinamic/node_modules/nyc/index.js:297:11)
    emit (/home/khoa/web/statinamic/node_modules/signal-exit/index.js:68:11)
    processEmit [as emit] (/home/khoa/web/statinamic/node_modules/signal-exit/index.js:143:5)
@jamestalmage
Copy link
Member

mock-fs is problematic. You can use mountfs to get around it. See avajs/ava#665

@thangngoc89
Copy link
Author

Thank you. I'll give it a go

@jamestalmage
Copy link
Member

Even following the linked instructions, nyc causes the repo to fail.

See https://github.com/thangngoc89/nyc-mock-fs

I'm stumped. @novemberborn ?

@jamestalmage
Copy link
Member

Seems promise-fs is messing things up somehow, even though the file system is fully mocked.

See thangngoc89/nyc-mock-fs#1.

Switching from promise-fs to just pify and fs fixes the problem.

@novemberborn
Copy link
Contributor

fs-promise uses mz which tries to load graceful-fs. Unfortunately nyc has a transitive dependency on graceful-fs as well. This means fs-promise ends up binding against the unmocked graceful-fs.

The pify approach is late-binding against fs.readFile, thus it gets to use the actual mocked filesystem.

I'd say this isn't an issue with nyc. It's other libraries doing strange things.

@jamestalmage
Copy link
Member

Thanks for the in depth research @novemberborn.

I'd say this isn't an issue with nyc. It's other libraries doing strange things.

Agreed.

@bcoe
Copy link
Member

bcoe commented Mar 28, 2016

This is definitely a danger with nyc having any library dependencies, I wonder if there should be a step where we clear the require-cache -- Isaacs told me this can be risky mind you, creating more opportunities for memory leaks.

@novemberborn
Copy link
Contributor

We might be better off reducing the dependencies we introduce into the child processes, possibly rewiring (à la proxyquire) any remaining dependencies so their bindings survive mocking.

@jamestalmage
Copy link
Member

Why not just shrinkwrap everything?

@novemberborn
Copy link
Contributor

@jamestalmage assuming npm doesn't dedupe the dependency shrinkwrap I guess that would work for nyc's dependency tree, aside from when the standard modules are mocked.

@bcoe
Copy link
Member

bcoe commented Mar 31, 2016

@novemberborn @jamestalmage perhaps we start bundling dependencies for nyc ... as much as I'm not a fan of this approach, it does stop singletons from breaking.

@jamestalmage
Copy link
Member

What is the downside? I understand there are implications for a normal library, but for one that is basically guaranteed only to be used as a dev-dependency I think those downsides are mitigated somewhat.

We probably should seek outside opinions.

@isaacs @sindresorhus

@sindresorhus
Copy link
Member

ESLint recently tried bundling, but encountered an issue with bundledDependencies and npm@2: eslint/eslint#5013

@bcoe
Copy link
Member

bcoe commented Apr 3, 2016

@thangngoc89 @novemberborn @jamestalmage give this branch a shot on a few of your repos:

npm i nyc@bundle-test

I've started bundling dependencies, it should also fix the issue with node 5.8.0.

@thangngoc89
Copy link
Author

@bcoe nice. Test passed in https://github.com/thangngoc89/nyc-mock-fs . I'll take another look at it with a real app at the end of the day

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

5 participants