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

Publish code coverage to Code Climate #26

Merged
merged 7 commits into from
Feb 7, 2018
Merged

Conversation

sgerrand
Copy link
Contributor

@sgerrand sgerrand commented Feb 6, 2018

💁 These changes add and configure code coverage output (via SimpleCov) and publish them to Code Climate.

@sgerrand sgerrand requested a review from bliof February 6, 2018 17:11
Copy link
Contributor

@bliof bliof left a comment

Choose a reason for hiding this comment

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

👍 I've changed couple of things => 👀

@@ -21,5 +21,6 @@ Gem::Specification.new do |s|

s.add_runtime_dependency 'bundler', ['>= 1.11.0', '< 1.17']

s.add_development_dependency 'rspec', '~> 3.4', '>= 3.4.0'
s.add_development_dependency 'rspec', '~> 3.7.0'
s.add_development_dependency 'simplecov', '~> 0.15.1'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙋 I deliberately didn't do this to ensure that SimpleCov wasn't loaded before it was required in spec_helper.rb.

Copy link
Contributor

@bliof bliof Feb 7, 2018

Choose a reason for hiding this comment

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

🤔 It would be loaded when you do require 'simplecov', right? There shouldn't be autoload for things in the gemspec (or bundler is doing something)

Copy link
Contributor

Choose a reason for hiding this comment

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

I've added a Simplecov.start in bin/hellgrid under the bundle/setup and it is not loaded:

./bin/hellgrid:11:in `<main>': uninitialized constant Simplecov (NameError)
Did you mean?  SimpleDelegator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you meant SimpleCov (note the case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There shouldn't be autoload for things in the gemspec (or bundler is doing something)

From memory, the gemspec line in the Gemfile loads any gems defined in the gemspec.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh.. still same thing:

./bin/hellgrid:11:in `<main>': uninitialized constant SimpleCov (NameError)
Did you mean?  SimpleDelegator

or if I remove the require in the spec_helper

An error occurred while loading ./spec/hellgrid/matrix_spec.rb.
Failure/Error: SimpleCov.start

NameError:
  uninitialized constant SimpleCov
  Did you mean?  SimpleDelegator
# ./spec/spec_helper.rb:1:in `<top (required)>'
# ./spec/hellgrid/matrix_spec.rb:1:in `require'

Copy link
Contributor

Choose a reason for hiding this comment

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

rubygems/bundler#2018 tl&dr point: if you don't require your things manually, the gem will fail when installed with gem install without bundler, so -> bundler only modifies the path and you do the requires

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Thanks for doing the research on this!

@@ -1,3 +1,4 @@
Gemfile.lock
hellgrid-*.gem
spec/tmp
coverage/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👏 Thanks!

@sgerrand sgerrand merged commit c3e4c0e into master Feb 7, 2018
@sgerrand sgerrand deleted the codeclimate-test-reporter branch February 7, 2018 10:17
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

2 participants