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

chore: add tests for #420 #429

Merged
merged 3 commits into from
Oct 30, 2016
Merged

chore: add tests for #420 #429

merged 3 commits into from
Oct 30, 2016

Conversation

bcoe
Copy link
Member

@bcoe bcoe commented Oct 29, 2016

@motiz88 here's a test of the happy-path for the new instrumentation logic. I'm going to go ahead and make sure this works on a Windows VM.

@bcoe
Copy link
Member Author

bcoe commented Oct 29, 2016

@motiz88 tests run great on v5 on my crumby Windows VM:

screen shot 2016-10-29 at 11 43 01 am

Anything you think I'm missing?

@bcoe
Copy link
Member Author

bcoe commented Oct 29, 2016

@motiz88 this feels like a step in the right direction, I think it puts you in a position to use this with babel itself?

My concern is it still doesn't solve the common babel-register case:

  • allowing nyc to instrument steps on the toes of babel-plugin-istanbul
  • without having nyc instrument, the files are never loaded, due to this logic:
  this.walkAllFiles(this.cwd, function (filename) {
    filename = path.resolve(_this.cwd, filename)
    _this.addFile(filename)
    var coverage = coverageFinder()
    var lastCoverage = _this.instrumenter().lastFileCoverage()
    if (lastCoverage) {
      filename = lastCoverage.path
    }
    if (lastCoverage && _this.exclude.shouldInstrument(filename)) {
      coverage[filename] = lastCoverage
    }
  })

any ideas as to how we might be able to delegate to babel-register, without explicitly requiring the file?

@bcoe
Copy link
Member Author

bcoe commented Oct 29, 2016

@motiz88 I've done some more digging, things are actually 99% of the way there; the babel-plugin-istanbul plugin populates everything except the magic value for files included via --all; I'm going to dig into why this might be the case.

@bcoe
Copy link
Member Author

bcoe commented Oct 30, 2016

@motiz88 of all things, this turned out to be an issue with babel's cache not playing nice with nyc. I've got --all working with babel-register now 👍

screen shot 2016-10-29 at 5 59 50 pm

@bcoe bcoe merged commit b90d26f into master Oct 30, 2016
@bcoe bcoe deleted the initial-coverage-tests branch October 30, 2016 01:13
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

1 participant