Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

Pipe #30

Closed
wants to merge 8 commits into from
Closed

Pipe #30

wants to merge 8 commits into from

Conversation

dotnetCarpenter
Copy link
Contributor

@dotnetCarpenter dotnetCarpenter commented Sep 6, 2016

This is my initial PR for reading piped reports - #29 . It needs a little more work and I would like to hear your opinion.

3 questions:

  1. What should the path be for piped reports?
  2. The check for if (files) ... will always be true. Why not remove it?
  3. What is the difference between lcov and coverage.json, except the data format? E.g. when is one generated over the other and is it by codecov-node or an 3th party? - just saw your answer in comment-244543227

@codecov-io
Copy link

codecov-io commented Sep 6, 2016

Codecov Report

Merging #30 into master will increase coverage by 0.54%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
+ Coverage   95.59%   96.13%   +0.54%     
==========================================
  Files          17       16       -1     
  Lines         227      233       +6     
  Branches       43       44       +1     
==========================================
+ Hits          217      224       +7     
+ Misses         10        9       -1
Impacted Files Coverage Δ
lib/codecov.js 93.54% <60%> (+1.17%) ⬆️
lib/services/localGit.js 100% <0%> (ø) ⬆️
lib/services/drone.js 100% <0%> (ø) ⬆️
lib/services/gitlab.js 100% <0%> (ø) ⬆️
lib/services/circle.js 100% <0%> (ø) ⬆️
lib/offline.js

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 3c8fb1b...d460065. Read the comment docs.

@dotnetCarpenter
Copy link
Contributor Author

@stevepeak For some reason the test goes stale after passing all tests. Not sure what is going on yet.

@dotnetCarpenter
Copy link
Contributor Author

@stevepeak finally it passes. But codecov doesn't seem to pick it up?

var timer = setTimeout(function() {
if (!readingPiped) {
codecov.upload(args);
process.exit();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this may be exiting before uploading is complete.

@stevepeak
Copy link
Contributor

The process seems to quite before uploading is started. You can see this in the build logs. I think the issue is beacuse of the pipe timeout/exit.

@eddiemoore
Copy link
Collaborator

Closing due to inactivity

@eddiemoore eddiemoore closed this Oct 23, 2017
@dotnetCarpenter
Copy link
Contributor Author

I'll revisit this in January.

@hadrienk
Copy link

hadrienk commented Mar 1, 2018

It would be just fine without the timeout IMO. Most CI services are getting slow depending on global usage. A timeout will only make the report fails inconsistently.

Let me know if you don't have time to work on this I could try to take over.

@dotnetCarpenter
Copy link
Contributor Author

@hadrienk thanks for taking over. I just don't have time ATM

@dotnetCarpenter
Copy link
Contributor Author

@hadrienk I vaguely remember having the timeout to distinguish between piped input and file input but I don't remember if I actually tested this or if it's the remnants of an issue with a different project I wrote, that has the same listening implementation.

@dotnetCarpenter
Copy link
Contributor Author

So I just need to remove the timeout? I should be able to find time for that soon.

@dotnetCarpenter
Copy link
Contributor Author

@hadrienk Checkout #92

@hadrienk
Copy link

hadrienk commented Mar 13, 2018 via email

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.

None yet

5 participants