Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Use the remapped code coverage summary for code coverage threshold check #499

Merged
merged 15 commits into from
Nov 15, 2018

Conversation

Blackbaud-PaulCrowder
Copy link
Member

No description provided.

@codecov-io
Copy link

codecov-io commented Nov 12, 2018

Codecov Report

Merging #499 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #499   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          54     54           
  Lines        1686   1711   +25     
  Branches      249    254    +5     
=====================================
+ Hits         1686   1711   +25
Flag Coverage Δ
#builder 100% <100%> (ø) ⬆️
#runtime 100% <ø> (ø) ⬆️
#srcapp 100% <ø> (ø) ⬆️
Impacted Files Coverage Δ
config/karma/shared.karma.conf.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7841b3...1b5950f. Read the comment docs.

return remapIstanbul.remap(collector.getFinalCoverage());
const newCollector = remapIstanbul.remap(collector.getFinalCoverage());

if (!remapCoverageSummary) {

Choose a reason for hiding this comment

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

Any concern on how this might affect multiple browsers? My concern is that the first result will be returned for all subsequent calls, even though they may be different browsers with different coverage.

'functions'
];

const threshold = getCoverageThreshold(skyPagesConfig);

Choose a reason for hiding this comment

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

I don’t believe this line is needed twice.


let coverageFailed;

keys.forEach(function (key) {

Choose a reason for hiding this comment

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

Any reason to not use fat arrow here?

@Blackbaud-PaulCrowder
Copy link
Member Author

@Blackbaud-BobbyEarl This should be ready for another look.

@Blackbaud-BobbyEarl
Copy link

Ha, guess I should've checked that I reviewed this yesterday. D'oh.

@Blackbaud-PaulCrowder Blackbaud-PaulCrowder merged commit 76f7239 into master Nov 15, 2018
@Blackbaud-PaulCrowder Blackbaud-PaulCrowder deleted the code-coverage-threshold-fix branch November 15, 2018 21:05
Blackbaud-MikitaYankouski pushed a commit to Blackbaud-MikitaYankouski/skyux-builder that referenced this pull request May 3, 2019
…eck (blackbaud#499)

* Use the remapped code coverage summary for code coverage threshold check

* Fixed lint rule

* Unit tests

* Removed redundant const

* Validate that done() called

* Be explicit about whether test should pass coverage threshold check

* Fixed lint error

* Report coverage inside _onWriteReport() since _onExit() doesn't get called during `watch`

* Minor refactoring

* Fixed linting error.

* Use function expression for consistency

* Changed `let` to`const` where appropriate
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants