krisleech / wisper

A micro library providing Ruby objects with Publish-Subscribe capabilities
3.27k stars 151 forks source link

'coveralls' missing as a build dependency in gemspec #182

Closed lelutin closed 3 years ago

lelutin commented 4 years ago

Hello,

I'm working on packaging wisper for debian and I'm getting rspec failures for all tests.

Since the Rakefile and spec_helper.rb files are making use of the coveralls library, I believe coveralls should somehow be mentioned as a build dependency in the gemspec. Without this, unit tests fail to run during package build.

I've worked around the issue by adding the build-deb manually in the debian package, so was just meaning to make a sign about this missing bit here.

If I'm not mistaken, this could be achieved by adding the following line in the Gem::Specification block in your .gemspec:

  gem.add_development_dependency "coveralls"

Cheers!

bmorrall commented 4 years ago

Coveralls is included in the Gemfile, which allows it to be built locally.

I'm curious, why are you trying to build a Debian package with the test suite included?

lelutin commented 4 years ago

@bmorrall the debian infrastructure runs test suites automatically when building packages whenever they are present. we like to configure them so they run properly: this way we can more easily see if a change in dependency libraries or ruby version has the effect of breaking functionality in some package.

krisleech commented 4 years ago

:thinking: Is it possible to do bundle install as part of the build step for the tests this will install all the development/test dependencies which are in Gemfile?

krisleech commented 4 years ago

There is a second option and that is something like:

begin
  require 'coveralls'
rescue LoadError
  # no-op
else
  Coveralls.wear!
end
lelutin commented 4 years ago

@krisleech so I sought out some feedback from fellow ruby packagers since I'm still very fresh in that set of tech skills. I should've known better, but ain't that always the case for something -- especially for beginners hehe. coveralls is not required at all for the purpose of running the test suite, so I could've patched it out of spec_helper.rb.

the patch you suggested in your last message seems to answer perfectly to that need, so if you and other contributors are comfortable merging that in, it would solve the issue for me. however it might mean that some developers won't update coverage information systematically since it won't be compulsory to run coveralls anymore. if some ppl are not super comfortable, I can patch it at packaging level.

geor-g commented 4 years ago

@lelutin Personally, I'm using env vars to be able to "opt-out" in such situations if packaging stuff for Debian.

krisleech commented 3 years ago

I'm happy with the rescue no-op solution if someone wants to submit a PR.