Feature profdata #99
Feature profdata #99
Conversation
…te/slather into feature-temp-profdata * 'feature-temp-profdata' of https://github.com/viteinfinite/slather: Remove useless file Fix tests Remove useless files Fix unit tests Add more precise path handling Add scheme and ignore Export coverage base info into modules Fix coveralls_spec Minor fixes in spec_helper Re-add coveralls support Re-add coveralls support Add tests [WIP] Refactor interface of coverage_file [WIP] Add unit tests [WIP] Add experimental support to swift
| raise StandardError, "No Coverage.profdata files found. Please make sure the \"Code Coverage\" checkbox is enabled in your scheme's Test action or the build_directory property is set." | ||
| end | ||
| xcode_path = `xcode-select -p`.strip | ||
| llvm_cov_command = File.join(xcode_path, "Toolchains/XcodeDefault.xctoolchain/usr/bin/llvm-cov show -instr-profile #{coverage_profdata} #{binary_file}") |
There was a problem hiding this comment.
Shouldn't we use xcode_path.shellescape in case the xcode_path contains spaces, parentheses, or other characters that needs escaping?
Probably same for coverage_profdata and binary_file btw, so maybe even use shelljoin?
|
Very interested in this being available, as our CI can't build coverage reports using the legacy gcno files anymore when building with Xcode 7 see Apple forums so I'm hoping to migrate to profdata files instead. What's missing before accepting this PR? How can we help making it mergable? Thx a lot! |
|
Those changes you mentioned with @viteinfinite @AliSoftware @mattdelves As an aside, obviously I'll continue to maintain this gem, but if anybody is ever eager to try out a new feature before it makes its way into the official Slather rubygem, you can always point your gem 'slather', :git => "https://github.com/viteinfinite/slather"That'll pull master off of the fork. You can also specify a |
|
Great news, thx! I thought about testing that branch today, but lost a lot of time trying to debug that Xcode 7 bug with gcno files instead, so maybe I'll have time next week, but if it's merged and released in the meantime that would be much better 👍 PS: thx for the awesome tool, I just switched from |
|
Getting an error when trying to generate HTML report |
|
Are there any plans to merge this PR or the other one (#92) soon? |
|
We need to get
|
…le CLANG_ENABLE_CODE_COVERAGE instead of GCC_GENERATE_TEST_COVERAGE_FILES + Print explicit error when missing parameter for "slather setup" TODO: improve Xcodeproj gem so my new Xcodeproj::XCScheme implementation also allows to set codeCoverageEnabled = YES in every xcscheme
This will need us to update the dependency against the xcodeproj gem to "~> 0.27" (which requires to bump the dependency on cocoapods as well)
|
@marklarr I've created a PR on mattdelves/slather#1 to escape Note: Ideally to make |
|
@AliSoftware Is this really required? The |
|
@mgrebenets the whole point of If you execute your tests in the CLI there is no real benefit of using |
|
@AliSoftware got it. |
|
I have tested this branch again lately, and I noticed that Slather's coverage reports are not the same as Here's what I get directly from Filename Regions Miss Cover Functions Executed
-----------------------------------------------------------------------
...usr/include/MacTypes.h 0 0 nan% 0 nan%
...sr/include/objc/objc.h 0 0 nan% 0 nan%
...r/include/sys/_types.h 0 0 nan% 0 nan%
...tchTests/AppDelegate.m 17 5 70.59% 7 57.14%
...DetailViewController.m 8 8 0.00% 4 0.00%
...MasterViewController.m 30 25 16.67% 10 40.00%
...MatchTests/Model.swift 1 1 0.00% 1 0.00%
...MatchTests/ObjCModel.m 1 0 100.00% 1 100.00%
...ixAndMatchTests/main.m 3 0 100.00% 1 100.00%
-----------------------------------------------------------------------
TOTAL 60 39 35.00% 24 41.67%And this is matching Xcode output one to one (ignore the system files, those don't really matter). But then I ask slather to convert profdata to Cobertura XML bundle exec slather coverage \
--input-format profdata \
--cobertura-xml \
--ignore "../**/*/Xcode*" \
--output-directory slather-report \
--scheme MixAndMatchTests \
MixAndMatchTests.xcodeprojAnd results are like this MixAndMatchTests/AppDelegate.m: 11 of 33 lines (33.33%)
MixAndMatchTests/DetailViewController.m: 9 of 23 lines (39.13%)
MixAndMatchTests/MasterViewController.m: 22 of 63 lines (34.92%)
MixAndMatchTests/Model.swift: 0 of 3 lines (0.00%)
MixAndMatchTests/ObjCModel.m: 3 of 3 lines (100.00%)
MixAndMatchTests/main.m: 5 of 5 lines (100.00%)
Test Coverage: 38.46%If you examine them closely, they don't really match to I assume there are some bugs with the logic that converts profile data to other formats. You can grab Xcode project with few shell scripts to reproduce the issue. |
* Change "--input-format" option of "slather setup" to "--format", more appropriate name in the context of setup * Supporting auto for `slather setup --format` and `slather coverage --input-format` which will use profdata if Xcode 7, gcov if earlier * Checking the validity of the parameters given to `--format` / `--input-format`
Fixes for your profdata support
|
@marklarr you can now check one of the 3 boxes ("Paths escaped") in your previous comment above 👍 The issue pointed out by @mgrebenets and the one about the HTML report will probably need more work though :-/ |
|
I would stick with using |
|
Taking a bit of a look at the tests, they seem to all be relating to a specific type of failure. That is, the integer value isn't being read correctly. An example of this is:
It seems odd to me that we are expecting the binary value 000000000000001, and getting the binary value 1000000000000000. Is it possible that Apple decided to change the format of their numbers in Swift 2? |
|
That was my initial thought about the binary stuff. What slather does though is run the gcov command which generates the files. Looks to me that the issue may be in gcov. |
|
Any news on this, anyone have any idea what's wrong or started to investigate? |
|
I can help with testing if needed. |
|
Sorry about the lack of comment on my behalf, having been moving house and job so not had the time to look at it. @yakimant feel free to take a look, from what I can tell it is a change in the gcov output though am unsure what the expected behaviour is. |
|
Hey for whats it worth: Codecov has been using the following Bash script to upload many of swift coverage reports: profdata=$(find ~/Library/Developer/Xcode/DerivedData -name 'Coverage.profdata' | head -1)
if [ -f "$profdata" ];
then
_dir=$(dirname "$profdata")
for _type in app framework xctest
do
_file=$(find "$_dir" -name "*.$_type" | head -1)
if [ "$_file" != "" ];
then
_proj=${_file##*/}
_proj=${_proj%."$_type"}
xcrun llvm-cov show -instr-profile "$profdata" "$_file/$_proj" > "$_type.coverage.txt" || true
fi
done
fi
|
|
@stevepeak. This reports are then uploaded to Codecov right? What if I want code coverage reports integrated into my build job summary, can I still achieve it with Codecov? For example Cobertura coverage report plugin for Jenkins gives me functionality like this:
All of that I get right in the build summary without the need to follow external links. This is exactly why I'm looking for a way to turn output of When I scanned through the |
|
@mgrebenets Yes, this is our uploading tool.
Codecov is SaaS product that archives these reports. It may be a great solution for you. Here is an example repository: https://codecov.io/gh/wikimedia/wikipedia-ios
This is not necessary if you are using Codecov. I do not have much insight into |
|
@mgrebenets On my project I get different output from And Xcode shows me: Check |
|
@mAu888 From what I understand, that is because the So, if you have two methods, |
|
@dbarden I see. Makes sense, as the progress from Xcode matches the output of running slather. |
|
@mAu888, we are generating the report and calculating the total using (llvm-cov report) Smth, like this: |
|
I get inconsistent results from The Both |
|
Whats the ETA on this or the other profdata PR being merged? |
|
@dyundt This still needs multiple fixes. For first we need slather to work with both ObjC and Swift projects. |
|
In your main target you need to set the Run your test and enjoy the 100% code coverage for the NB: With this, you cannot test swift classes with objective-C, or at least I didn't find how to import the tested module in the test class. Hope this help |
|
@GUL- To test Swift classes with Objective-C code I would have to import Swift umbrella header (the I think I can live with it. If I have a Swift class, why not write tests for it with Swift too. |
|
I keep getting the output below from this PR.... bundle exec slather coverage --input-format profdata --simple-output --ignore "..///Xcode" --output-directory slather-report --scheme ${SCHEME} ${PROJECT} And if I try slather setup, I get... |
|
What seems to be the prominent branch that we are using for this profdata discussion? We should close one and use just one for this topic. |
|
For anyone interested, I've rebased this branch and attempted to fix up the html output for prof data over at https://github.com/dhardiman/slather/tree/feature-profdata. I've not raised a pull request as it doesn't attempt to fix the gcov integration with Xcode 7, and is probably not well enough tested (my ruby skills are weak). From what I can tell, they've changed the endianness (or something) of the gcda output, which seems to break gcov itself. Which makes me think supporting gcov with Xcode 7 is probably not worth worrying about any more. So my branch serves my purposes, and if it helps anyone else, that's great. |
|
@dhardiman I agree with you. One of the next versions of slather should stop supporting gcov. |
|
I've continued working on the integration of profdata into slather. More commits are available in the PR #92. |
|
@viteinfinite Ok. I will close this PR then in favor of #92 |
This is a simple rebase of the work that @viteinfinite has done to get xcode7 coverage reports into slather.
Would be great for people to get this rebase looked over so that the functionality can be merged in.
This should bring #92 up to date so that it can be merged in.