uses_tzinfo / uses_gem patches require eyeballs

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?




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.