Skip to content

Conversation

@bcoe
Copy link
Member

@bcoe bcoe commented Apr 19, 2017

--cache was not being respected for the top-level process, this resulted in coverage information being mapped differently in the parent process than the child-processes, resulting in a fluctuating coverage percentage.

I pulled the source-map logic into its own library while debugging this issue, because it made things simpler to follow.

@bcoe bcoe requested a review from JaKXz April 19, 2017 08:17
Copy link
Member

@JaKXz JaKXz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a quick 👀 but I can review again in the evening.

var libCoverage = require('istanbul-lib-coverage')
var libSourceMaps = require('istanbul-lib-source-maps')
var fs = require('fs')
var path = require('path')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have consts elsewhere, but vars here. I'm guessing we're going to Node v4+ soon so I'd like to be consistent :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 will make more consistent usage of const.

index.js Outdated
disableCache: !this.enableCache,
// when running --all (in the parent process) we should
// not load source from cache.
disableCache: !(this.cache && this.config.isChildProcess),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add a test for this statement

@bcoe bcoe merged commit ff73b18 into master Apr 20, 2017
@bcoe bcoe deleted the 537-fix branch April 20, 2017 21:13
jimthedev referenced this pull request in commitizen/cz-cli May 24, 2018
This Pull Request updates dependency [nyc](https://github.com/istanbuljs/nyc) from `v10.0.0` to `v10.3.2`



<details>
<summary>Release Notes</summary>

### [`v10.3.2`](https://github.com/istanbuljs/nyc/blob/master/CHANGELOG.md#&#8203;1100httpsgithubcomistanbuljsnyccomparev1032v1100-2017-05-31)
[Compare Source](istanbuljs/nyc@e062a86...v10.3.2)
##### Bug Fixes

* add support for ES6 modules ([f18f780](istanbuljs/nyc@f18f780))
##### Features

* allow .nycrc.json ([#&#8203;580](`https://github.com/istanbuljs/nyc/issues/580`)) ([a1a457f](istanbuljs/nyc@a1a457f))
* upgrade to version of yargs with support for presets ([33829b8](istanbuljs/nyc@33829b8))
##### BREAKING CHANGES

* new version of find-up requires dropping 0.10/0.12 support (which we had already been planning).
#### [10.3.2](istanbuljs/nyc@v10.3.1...v10.3.2) (2017-05-05)
##### Bug Fixes

* we should not create a cache folder if cache is false ([#&#8203;567](`https://github.com/istanbuljs/nyc/issues/567`)) ([213206f](istanbuljs/nyc@213206f))
#### [10.3.1](istanbuljs/nyc@v10.3.0...v10.3.1) (2017-05-04)
##### Bug Fixes

* introduced a bug that resulted in source-maps not being loaded approriately on second test run ([#&#8203;566](`https://github.com/istanbuljs/nyc/issues/566`)) ([1bf74fd](istanbuljs/nyc@1bf74fd))

---

### [`v10.3.1`](https://github.com/istanbuljs/nyc/blob/master/CHANGELOG.md#&#8203;1032httpsgithubcomistanbuljsnyccomparev1031v1032-2017-05-05)
[Compare Source](istanbuljs/nyc@v10.3.0...v10.3.1)
##### Bug Fixes

* we should not create a cache folder if cache is false ([#&#8203;567](`https://github.com/istanbuljs/nyc/issues/567`)) ([213206f](istanbuljs/nyc@213206f))

---

### [`v10.3.0`](https://github.com/istanbuljs/nyc/blob/master/CHANGELOG.md#&#8203;1030httpsgithubcomistanbuljsnyccomparev1020v1030-2017-04-29)
[Compare Source](istanbuljs/nyc@55e826d...v10.3.0)
##### Bug Fixes

* source-maps were not being cached in the parent process when --all was being used ([#&#8203;556](`https://github.com/istanbuljs/nyc/issues/556`)) ([ff73b18](istanbuljs/nyc@ff73b18))
##### Features

* add support for --no-clean, to disable deleting raw coverage output ([#&#8203;558](`https://github.com/istanbuljs/nyc/issues/558`)) ([1887d1c](istanbuljs/nyc@1887d1c))

---

### [`v10.2.2`](istanbuljs/nyc@2ff8f3b...2ff8f3b)
[Compare Source](istanbuljs/nyc@2ff8f3b...2ff8f3b)


---

### [`v10.2.1`](istanbuljs/nyc@v10.2.0...2ff8f3b)
[Compare Source](istanbuljs/nyc@v10.2.0...2ff8f3b)


---

### [`v10.2.0`](https://github.com/istanbuljs/nyc/blob/master/CHANGELOG.md#&#8203;1030httpsgithubcomistanbuljsnyccomparev1020v1030-2017-04-29)
[Compare Source](istanbuljs/nyc@43535f9...v10.2.0)
##### Bug Fixes

* source-maps were not being cached in the parent process when --all was being used ([#&#8203;556](`https://github.com/istanbuljs/nyc/issues/556`)) ([ff73b18](istanbuljs/nyc@ff73b18))
##### Features

* add support for --no-clean, to disable deleting raw coverage output ([#&#8203;558](`https://github.com/istanbuljs/nyc/issues/558`)) ([1887d1c](istanbuljs/nyc@1887d1c))

---

### [`v10.1.2`](https://github.com/istanbuljs/nyc/blob/master/CHANGELOG.md#&#8203;1012httpsgithubcomistanbuljsnyccomparev1011v1012-2017-01-18)
[Compare Source](istanbuljs/nyc@e46335f...v10.1.2)
##### Bug Fixes

* revert defaulting to empty file-coverage report, this caused too many issues ([25aec77](istanbuljs/nyc@25aec77))

---

### [`v10.1.0`](https://github.com/istanbuljs/nyc/blob/master/CHANGELOG.md#&#8203;1020httpsgithubcomistanbuljsnyccomparev1010v1020-2017-03-28)
[Compare Source](istanbuljs/nyc@8f7af3a...v10.1.0)
##### Bug Fixes

* fix bug related to merging coverage reports see [#&#8203;482](`https://github.com/istanbuljs/nyc/issues/482`) ([81229a0](istanbuljs/nyc@81229a0))
* revert defaulting to empty file-coverage report, this caused too many issues ([25aec77](istanbuljs/nyc@25aec77))
##### Features

* allow babel cache to be enabled ([#&#8203;517](`https://github.com/istanbuljs/nyc/issues/517`)) ([98ebdff](istanbuljs/nyc@98ebdff))
* exclude the coverage/ folder by default 🚀 ([#&#8203;502](`https://github.com/istanbuljs/nyc/issues/502`)) ([50adde4](istanbuljs/nyc@50adde4))
* upgrade to version of yargs with extend support ([#&#8203;541](`https://github.com/istanbuljs/nyc/issues/541`)) ([95cc09a](istanbuljs/nyc@95cc09a))
#### [10.1.2](istanbuljs/nyc@v10.1.1...v10.1.2) (2017-01-18)
##### Bug Fixes

* revert defaulting to empty file-coverage report, this caused too many issues ([25aec77](istanbuljs/nyc@25aec77))
#### [10.1.1](istanbuljs/nyc@v10.1.0...v10.1.1) (2017-01-18)
##### Bug Fixes

* fix bug related to merging coverage reports see [#&#8203;482](`https://github.com/istanbuljs/nyc/issues/482`) ([81229a0](istanbuljs/nyc@81229a0))

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).
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.

3 participants