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

Add support for reading LCOV files #191

Merged
merged 3 commits into from Jan 2, 2019
Merged

Add support for reading LCOV files #191

merged 3 commits into from Jan 2, 2019

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Dec 14, 2018

Julia v1.1 is slated to have a new, more flexible code-coverage format (JuliaLang/julia#30381). This is the prep-work (e.g. helper functions) for utilizing that information.

@ararslan
Copy link
Member

Looks like that change likely won't make it into 1.1.

@vtjnash vtjnash force-pushed the jn/read-lcov branch 2 times, most recently from 05b890f to deb6f4b Compare December 14, 2018 22:25
@codecov-io
Copy link

codecov-io commented Dec 14, 2018

Codecov Report

Merging #191 into master will increase coverage by 6.36%.
The diff coverage is 96.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #191      +/-   ##
==========================================
+ Coverage   67.22%   73.59%   +6.36%     
==========================================
  Files           6        6              
  Lines         296      337      +41     
==========================================
+ Hits          199      248      +49     
+ Misses         97       89       -8
Impacted Files Coverage Δ
src/Coverage.jl 86.79% <100%> (+3.81%) ⬆️
src/codecovio.jl 88.7% <100%> (+7.55%) ⬆️
src/lcov.jl 86.79% <94.44%> (+16.2%) ⬆️
src/memalloc.jl 66.66% <0%> (+8.33%) ⬆️

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 5e8e864...94a460e. Read the comment docs.

@vtjnash
Copy link
Member Author

vtjnash commented Dec 14, 2018

I think we'll get it in. But regardless, the only part of this that depends on it is the sample usage section of the README.

@vtjnash
Copy link
Member Author

vtjnash commented Dec 18, 2018

@fingolfin You seem to be the current de-facto maintainer—does this look good to you?

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Manifest.toml Show resolved Hide resolved
src/Coverage.jl Show resolved Hide resolved
test/data/Coverage.jl Outdated Show resolved Hide resolved
REQUIRE Outdated Show resolved Hide resolved
test/data/Coverage.jl Outdated Show resolved Hide resolved
test/data/Coverage.jl.cov Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
src/lcov.jl Outdated Show resolved Hide resolved
end

"""
readfolder(folder) -> Vector{FileCoverage}
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick, but: should it be read_file and read_folder to match process_file and process_folder?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was matching the existing LCOV.writefile with the name LCOV.readfile, and the general base guidelines to avoid underscores. I agree it would be nice to be more consistent, though I'm not sure how to accomplish that.

Copy link
Member

Choose a reason for hiding this comment

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

OK, I see -- yeah, when two differing "conventions" clash, this is always a bit nasty. In any case, it doesn't matter much, I was just wondering.

test/runtests.jl Show resolved Hide resolved
@vtjnash
Copy link
Member Author

vtjnash commented Jan 1, 2019

@fingolfin updated per review comments, is this good now?

@fingolfin
Copy link
Member

Thank you, this looks good to me now!

@fingolfin fingolfin merged commit 6daa007 into master Jan 2, 2019
@fingolfin fingolfin deleted the jn/read-lcov branch January 2, 2019 11:07
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

4 participants