Don't include any test files in the gem. #1345

Merged
merged 1 commit into from Feb 25, 2014

Projects

None yet

3 participants

@myronmarston
RSpec member

I don't think anyone actually relies on that
feature and it makes the published size of the
gem more than twice as big (192 KB vs 90 KB).

Anyone got a reason we shouldn't do this?

/cc @JonRowe @xaviershay @alindeman @samphippen @soulcutter

@myronmarston myronmarston Don't include any test files in the gem.
I don't think anyone actually relies on that
feature and it makes the published size of the
gem more than twice as big (192 KB vs 90 KB).
59d617d
@xaviershay
RSpec member

I kind of like being able to gem unpack or bundle show and start screwing around with the gem. I did have a slight preference for including it.

But then 100Kb is a lot of test files...

Unrelated: I remember some possible FUD about using backticks in gemfile being a bad idea because it slowed down gem loading significantly. Makes sense, since everytime the spec is loaded it needs to shell out. We should probably look at removing that.

@xaviershay
RSpec member

Ah this is what I was thinking of: http://tenderlovemaking.com/2011/12/05/profiling-rails-startup-with-dtrace.html This is an old post, may have already been addressed by bundler? I think only relevant when using the gem from git?

@JonRowe
RSpec member

I'd be ok with trimming down the files we send out in the gem.

@myronmarston
RSpec member

I kind of like being able to gem unpack or bundle show and start screwing around with the gem.

I view and edit the source code of gems all the time (typically using bundle show), and this won't prevent you from doing that. It will, however, prevent you from reading or running the tests when you do that. If you want the tests, you'll have to go clone the repo off github.

Do you ever do that? I've personally never done it or heard of anyone doing it.

IMO, I think the tradeoff is worth it. Having each gem be half the size (assuming we apply this to each repo) will cut down on rubygems.org's S3 costs--probably not substantially, but given that RSpec has ~50 million total gem downloads (summing across the gems) I think it can add up. It also makes installation faster for each and every user.

@xaviershay, how strongly do you feel about the need to keep the test files in the released gem, given you can source the gem off github in your gemfile if you want the tests or clone it yourself?

Unrelated: I remember some possible FUD about using backticks in gemfile being a bad idea because it slowed down gem loading significantly. Makes sense, since everytime the spec is loaded it needs to shell out. We should probably look at removing that.

Funny you mention this--I was actually thinking about this as well when I edited the gemspec for this issue! As you later mentioned, the shelling out only comes into affect when not using the released gem (as gem build compiles the gemspec into YAML), and even when you source the gems off github, if you use --standalone it avoids bundler reading the gemspec at runtime and thus would cut out the shelling out cost.

In some other gems I've switched away from backticks:

https://github.com/seomoz/interpol/blob/master/interpol.gemspec#L11-L14
https://github.com/seomoz/qless/blob/master/qless.gemspec#L27-L31

...but I've found it to be more of a maintenance burden than the backticks. The backticks "just work", so my instinct is to leave it.

@JonRowe
RSpec member

It'd be nice to have back ticks for dev, then compiled down when releasing...

@myronmarston
RSpec member

It'd be nice to have back ticks for dev, then compiled down when releasing...

That's what gem build does (and has always done). The gemspec in a realeased gem is static YAML, not ruby code.

@JonRowe
RSpec member

Then I don't think it's a problem to have a slightly slower time for people using git dependencies...

@xaviershay
RSpec member

Agree with everything above, let's remove.

@myronmarston
RSpec member

Cool. I want to keep this PR open until we've made PRs for the other gems that do the same thing so we don't forget. Then we can merge 'em all.

@JonRowe
RSpec member

So that's the other 3 core gems... Do we do rspec-rails too?

@myronmarston
RSpec member

Sure, why not?

@JonRowe JonRowe referenced this pull request in rspec/rspec-rails Feb 25, 2014
Merged

Don't include any test files in the gem. #942

@JonRowe JonRowe merged commit 80a1bea into master Feb 25, 2014

1 check passed

Details default The Travis CI build passed
@JonRowe JonRowe deleted the smaller-built-gem-size branch Feb 25, 2014
@JonRowe
RSpec member

S'all of them right?

@myronmarston
RSpec member

All the main gems. There are the spinoffs like rspec-its but they are tiny and it's not worth the effort there, IMO.

@myronmarston myronmarston referenced this pull request in octokit/octokit.rb Mar 5, 2014
Closed

Request to include tests in the gem #445

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