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

fix: respect cache flag in parent process #556

Merged
merged 10 commits into from
Apr 20, 2017
Merged

fix: respect cache flag in parent process #556

merged 10 commits into from
Apr 20, 2017

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.

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.

2 participants