Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

Generate slather/Coveralls test coverage stats#434

Merged
appleguy merged 8 commits intofacebookarchive:masterfrom
dasmer:master
Sep 18, 2015
Merged

Generate slather/Coveralls test coverage stats#434
appleguy merged 8 commits intofacebookarchive:masterfrom
dasmer:master

Conversation

@dasmer
Copy link
Copy Markdown
Contributor

@dasmer dasmer commented Apr 21, 2015

  • Modify project file to generate code coverage metrics
  • Add step to .travis.yml to install slather and send metrics to Coveralls
  • Add .slather.yml to configure how we send metrics to Coveralls.

cc: @modocache (https://twitter.com/modocache/status/590236557933678592)

Coveralls will need to enabled for facebook/AsyncDisplayKit. It's already enabled for dasmer/AsyncDisplayKit, and shows a coverage of 44%.

Comment thread .slather.yml
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Newline? :trollface:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a new line :octocat:

@modocache
Copy link
Copy Markdown

Looks like a legitimate build failure in https://travis-ci.org/facebook/AsyncDisplayKit/jobs/59346360, at the "Link Life Without CocoaPods" step:

ld: warning: directory not found for option '-L/Users/nadi/src/AsyncDisplayKit/build/Debug-iphoneos'
Undefined symbols for architecture x86_64:
  "_llvm_gcda_emit_arcs", referenced from:
      ___llvm_gcov_writeout in libAsyncDisplayKit.a(_ASDisplayLayer.o)
      ___llvm_gcov_writeout in libAsyncDisplayKit.a(ASDataController.o)
      ___llvm_gcov_writeout in libAsyncDisplayKit.a(ASTextNodeTextKitHelpers.o)
      ___llvm_gcov_writeout in libAsyncDisplayKit.a(ASTableView.o)
      ___llvm_gcov_writeout in libAsyncDisplayKit.a(ASRangeHandlerRender.o)
      ___llvm_gcov_writeout in libAsyncDisplayKit.a(ASCollectionView.o)
      ___llvm_gcov_writeout in libAsyncDisplayKit.a(ASTextNodeWordKerner.o)
      ...

I think I've seen this stack trace before, it may have been when GCC_GENERATE_TEST_COVERAGE_FILES wasn't properly set, or something.

@dasmer
Copy link
Copy Markdown
Contributor Author

dasmer commented Apr 21, 2015

@modocache Yep I missed Generate Test Coverage and Instrument Program Flow to YES for Life Without CocoaPods.xcodeproj. Looks like Travis is passing now!

@modocache
Copy link
Copy Markdown

@dasmer I've enabled Coveralls for facebook/AsyncDisplayKit. If you add a commit to this that adds a badge to the README, that'll kick off the Travis build and we can make sure the metrics are sent to https://coveralls.io/r/facebook/AsyncDisplayKit. 🙇

@dasmer
Copy link
Copy Markdown
Contributor Author

dasmer commented Apr 22, 2015

@modocache Done. Looks like metrics are being sent 📫

@appleguy
Copy link
Copy Markdown
Contributor

@dasmer & @modocache, thanks for collaborating on this :). I haven't used these tools before, and while the data they provide is certainly an interesting new metric to track and drive, my primary concern is added complication to the build system. Are you willing to support this if issues arise?

On test coverage, I'm glad to see tests included in many external PRs and think that coverage could be improved significantly with a focused effort to write tests for "ASDK 1.0" code. I have a few other issues that are higher priority for my ASDK time, but would be excited to review an merge any test-focused diffs!

@appleguy
Copy link
Copy Markdown
Contributor

I'm going to take the leap with you, guys. This tool seems pretty cool—I just got time to sign up for an account.

I would love to see test coverage increase in ASDK. If we can get this into a merge-able state, I'll take the diff as soon as I see it :)

@appleguy
Copy link
Copy Markdown
Contributor

appleguy commented Jul 8, 2015

We've continued to make progress on increasing code coverage with the new snapshot tests for layout and expanded ASTableView and ASCollectionView testing, with more to come!

@dasmer, @modocache do you have time to fix the likely-minor merge conflicts here? I'm not familiar with the proper setup or testing procedures here, but would like to get this closed out.

@dasmer
Copy link
Copy Markdown
Contributor Author

dasmer commented Jul 9, 2015

@appleguy I just merged master and it looks like coverage has increased by ~14%.
Coverage Status

@dasmer
Copy link
Copy Markdown
Contributor Author

dasmer commented Jul 15, 2015

@appleguy Does anything else need to be done here before merge?

@appleguy
Copy link
Copy Markdown
Contributor

@dasmer so sorry I fell behind on this — I started "visually ignoring" the PR as one that was stalled, but you had actually replied to my second comment! Email notifications weren't working for me at the time, and it's really stupid that the GitHub website's PR page doesn't clearly highlight PRs with unread comments.

I would love to merge this, and in fact, would find it exceptionally useful if we could make it happen in the next 12-24hrs...In about 25hrs I'll be onstage at NSSpain talking about ASDK 2.0, and wanted to make a point about the importance of unit testing & value of community members who have been adding tests + test infrastructure.

Right now it's not merging cleanly. If you can fix that, I'll merge it as soon as I see it (GMT time zone). If you want to do another re-sync with your repository, I'd be curious to see the current coverage, which I suspect is incrementally higher.

@dasmer
Copy link
Copy Markdown
Contributor Author

dasmer commented Sep 16, 2015

@appleguy No worries. I merged master. It looks like a test is failing now, although it seems unrelated. Maybe try re-running tests?

@appleguy
Copy link
Copy Markdown
Contributor

Sweet! Yeah, Travis is such a disappointment. Re-running now.

@nguyenhuy
Copy link
Copy Markdown
Contributor

Seems like the failing test is the one I've just added. I will take a look at it later on. If it isn't wrong then it's unstable. And neither case is acceptable.

@dasmer
Copy link
Copy Markdown
Contributor Author

dasmer commented Sep 16, 2015

Looks like tests are still failing. Please let me know if there's anything I can do to help!

@nguyenhuy
Copy link
Copy Markdown
Contributor

Tests are not only failing in this PR but all of others. I submitted #661 to disable the buggy test. It doesn't fail locally, at least for me. I will debug it early next week. But if you can do that before me, please do. Thanks :)

@nguyenhuy
Copy link
Copy Markdown
Contributor

Green lights. Good to go 👯 👯 👯

appleguy added a commit that referenced this pull request Sep 18, 2015
Generate slather/Coveralls test coverage stats
@appleguy appleguy merged commit 2ef40e1 into facebookarchive:master Sep 18, 2015
@appleguy
Copy link
Copy Markdown
Contributor

Hooray!! Super cool :)

@appleguy
Copy link
Copy Markdown
Contributor

Because not all builds run the tests, the overall number is confused (sometimes showing 0%), but it seems to be working if you dig a little bit deeper. Great tool!

https://coveralls.io/github/facebook/AsyncDisplayKit

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants