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: make __coverage__ the default approach we advocate for ES2015 coverage #268

Merged
merged 8 commits into from
Jun 13, 2016

Conversation

bcoe
Copy link
Member

@bcoe bcoe commented Jun 5, 2016

related to #266

The reason that setting include = / makes __coverage__ compatible with nyc is that / results in no files matching shouldInstrumentFile().

When nyc is configured with a rule that does match shouldInstrumentFile() it results in the source-map information populated by __coverage__ being clobbered, breaking output.

Rather than relying on this side-effect I propose that we do the following:

  • add a configuration option that allows source-maps to be turned off in nyc.
  • add a noop instrumenter so that we can skip having nyc instrument files (__coverage__ handles this).
  • pull nyc's shouldInstrumentFile logic into a module that can be used by both nyc and __coverage__, this way folks can opt to use nyc's include/exclude functionality, while using __coverage__ for instrumentation.
  • update nyc's documentation, documenting the usage of __coverage__ for ES2015 instrumentation.
  • direct folks in Better ES2015 Support #266 towards using __coverage__ as a fix for their test-coverage issues.

@jamestalmage @dtinth does this sound reasonable?

@bcoe bcoe merged commit 9fd2295 into master Jun 13, 2016
@bcoe bcoe deleted the __coverage__ branch June 13, 2016 03:52
@novemberborn
Copy link
Contributor

Hey @bcoe, apologies for the radio silence, I've been traveling 😄

This makes sense, though I wonder if we can shove more behavior into the instrumenter. nyc's default logic could take care of the source maps, for instance. We could also power --all and the writing of the coverage data to disk.

That would allow tools like copenhagen (if I get a chance to work on it) to plug in to nyc more neatly.

@bcoe
Copy link
Member Author

bcoe commented Jun 13, 2016

@novemberborn happy to gradually pull more logic into sub-modules, being mindful of the logic that will probably be shared across instrumentation libraries, e.g., in the case of --all, I imagine the iteration logic would remain fairly consistent, but a replacement instrument would be for asking the actual question.

Could you do me a favor and take nyc@next for a spin on a few of your projects, the __coverage__ logic will not be fully functional until @dtinth lands:

dtinth/babel-plugin-__coverage__#35

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.

None yet

2 participants