uses_tzinfo / uses_gem patches require eyeballs

rubyonrails-core
1

Hi all,

I added a ticket + tiny patch to stop actionpack giving errors when the tzinfo gem isn’t present[1] (it’s essentially the same as changeset 8808 [2] except to cover the tests added in changeset 8883 [3]).

I then got to looking around and found ticket 10945 [4], which suggests replacing the uses_mocha and uses_tzinfo methods with a generic uses_gem method. This seems like a good idea, so I upgraded the patch to apply to head and cover the same ground as the ticket I just added (#11399).

Obviously only one of these patches should be applied as they cover the same ground, so I (and 10945’s original author no doubt) would appreciate some eyeballs on them to see which seems the best. 11399 is tiny, but I think 10945 is better.

Also, I’ve +1’d 10945 which seems like bad form, as it now has a patch that I wrote. As it appears you can’t edit comments you’ve made and as a matter of principal should I -1 it as well to bring it back on an even keel?

Cheers,

Muz

[1] http://dev.rubyonrails.org/ticket/11399 [2] http://dev.rubyonrails.org/changeset/8808 [3] http://dev.rubyonrails.org/changeset/8883 [4] http://dev.rubyonrails.org/ticket/10945

2

Good catch on the ActionPack error.

I think having an underlying uses_gem method is good for keeping things DRY (see activesupport test/abstract_unit.rb), but I still think having uses_tzinfo/uses_mocha methods makes sense, because that allows us to declare the gem version dependency once per project test suite (instead of repeating it for every wrapped test, as in #10945.)

Given that, #10945's API looks like a bit of overkill for our current needs. For now, I think we can just copy over the active support uses_ methods to each project that needs them. This will also ensure that we're requiring to the same versions of Mocha and Tzinfo across all the projects.

Fyi, no need to require both Mocha and Stubba (as we're doing in ActionPack) if we require a recent version of Mocha.