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

Make sure the error is thrown and reported #296

Merged
merged 1 commit into from
Nov 12, 2019
Merged

Conversation

Colin-Stone
Copy link
Member

Signed-off-by: Colin Stone 30794003+Colin-Stone@users.noreply.github.com
The error needs to be rethrown to be reported.

Signed-off-by: Colin Stone <30794003+Colin-Stone@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Nov 12, 2019

Codecov Report

Merging #296 into master will decrease coverage by 0.27%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #296      +/-   ##
==========================================
- Coverage    91.5%   91.22%   -0.28%     
==========================================
  Files          15       15              
  Lines        2389     2098     -291     
  Branches      452      368      -84     
==========================================
- Hits         2186     1914     -272     
+ Misses        202      183      -19     
  Partials        1        1
Impacted Files Coverage Δ
src/extension.ts 86.6% <100%> (-2.38%) ⬇️
src/ProfileLoader.ts 100% <0%> (ø) ⬆️
src/Profiles.ts 100% <0%> (+12.59%) ⬆️

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 f65b20b...073dbff. Read the comment docs.

@Colin-Stone
Copy link
Member Author

Code coverage is still appearing a little wacky. The junit tests coverage is actually a little better as I have added one new line which is covered by testing.

@zFernand0
Copy link
Member

zFernand0 commented Nov 12, 2019

If we look at the history of the extension.ts file it would seem that codecov is reporting a different number of lines tracked on the file fluctuating between 814 and 1017. (See proof below)

Should we submit an issue to @codecov?

073dbff
image

image

f65b20b
image

image

Notice how it tracks the empty line 1515 in the parent commit (f65b20b) and not the corresponding line 1516 in the current commit (073dbff)

Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

Thanks for surfacing the error and adding the test : )
👍

@Colin-Stone
Copy link
Member Author

Should we submit an issue to @codecov?

Thanks for the review. Are we seeing similar codecov issues with cli?. I was going to ask if we need to create an example for them but I think they can see this so yes probably worth raising an issue

@Colin-Stone Colin-Stone merged commit 221658a into master Nov 12, 2019
@Colin-Stone Colin-Stone deleted the dataseterror branch November 12, 2019 13:57
@zFernand0
Copy link
Member

Possibly related issue on codecov-batch: codecov/codecov-bash#83
Similar issue on another project: zephyrproject-rtos/zephyr#13608

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