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

Projects
None yet
5 participants
@maul-esel
Contributor

maul-esel commented Sep 12, 2013

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

This comment has been minimized.

coveralls commented Sep 12, 2013

Coverage Status

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

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Sep 12, 2013

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

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Sep 12, 2013

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

@parkr

This comment has been minimized.

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

This comment has been minimized.

Contributor

maul-esel commented Sep 13, 2013

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

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Sep 14, 2013

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

This comment has been minimized.

coveralls commented Sep 14, 2013

Coverage Status

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

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Sep 15, 2013

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

This comment has been minimized.

@parkr

parkr Sep 15, 2013

Member

lol 鉂わ笍

@parkr

This comment has been minimized.

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

Merge pull request #1539 from maul-esel/coveralls
Add support for coveralls.io coverage reports

@mattr- mattr- merged commit 52a39db into jekyll:master Sep 17, 2013

1 check passed

default The Travis CI build passed
Details

mattr- added a commit that referenced this pull request Sep 17, 2013

@maul-esel maul-esel deleted the maul-esel:coveralls branch Sep 17, 2013

@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.