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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for coveralls.io coverage reports #1539

Merged
merged 6 commits into from Sep 17, 2013

Conversation

maul-esel
Copy link

OK, it's true, those badges in the README might be a little obsession of mine. Nevertheless, it occurs more often than it should that some behaviour in the jekyll code is accidentally broken, yet no test goes 馃挜 like it should - because there is no test for that particular behaviour. In most cases it's detected before merge, but there are also cases that go into the next release and then 馃挬 goes down.

So, while it is not necessary to test every single line and a coverage of 100,000000 % is surely not necessary, some visible report on it can probably help to improve jekyll's (already awesome) testing suite. This PR adds support for coverage analysis during Travis-CI builds and loading them up to coveralls.io (provided @mojombo signs in there with his github account).

See https://coveralls.io/r/maul-esel/jekyll for the current reports of this branch.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling aa10f8c on maul-esel:coveralls into * on mojombo:master*.

@maul-esel
Copy link
Author

Huh cool, didn't even know it does that. One more pro argument.

@maul-esel
Copy link
Author

Funny, just did this and now: Test coverage Travis-CI + CodeCilmate. Need to check this out.

@parkr
Copy link
Member

parkr commented Sep 13, 2013

So much cruft with the RUBY_VERSION and the ENVs! Argh! Not sure how I feel about this yet. :/

@maul-esel
Copy link
Author

Me neither. Let's check out that CodeClimate stuff first, I think that could be integrated much better.

@maul-esel
Copy link
Author

OK, it seems CodeClimate test coverage only works if you have a CodeClimate account 馃挵. So probably not worth it 馃槶.

As to all the ENV: looking at it again, we could possibly reduce that quite a bit: always run coverage, but only send it if ENV["TRAVIS"]. The slowdown is too small to notice anyway. The RUBY_VERSION however seems non-optional, because it simply doesn't work for 1.8.7.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b26327f on maul-esel:coveralls into * on mojombo:master*.

@maul-esel
Copy link
Author

So, I removed the ENV conditions as far as possible.

However, update on CodeClimate: they're working on support for open-source repos, though it might be some time (reference) and they're also working on the ability to combine coverage results from cucumber and unit tests, which jekyll needs.

I really think that on a long-term basis, using CodeClimate is better because having both their features combined next to each other can surely help a lot. Also, it wouldn't require using another service. So IMO the only question left is if we

  1. want to use coveralls as an intermediate solution 鉃★笍 merge
  2. or wait for CodeClimate 鉃★笍 close
  3. or don't want it at all 鉃★笍 close

@@ -1,2 +1,2 @@
source 'https://rubygems.org'
gemspec
gemspec
Copy link
Member

Choose a reason for hiding this comment

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

lol 鉂わ笍

@parkr
Copy link
Member

parkr commented Sep 15, 2013

I'd be OK with this! @mattr-?

mattr- added a commit that referenced this pull request Sep 17, 2013
Add support for coveralls.io coverage reports
@mattr- mattr- merged commit 52a39db into jekyll:master Sep 17, 2013
mattr- added a commit that referenced this pull request Sep 17, 2013
@maul-esel maul-esel deleted the coveralls branch September 17, 2013 11:47
@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017
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.

None yet

5 participants