mgomes / api_auth

HMAC authentication for Rails and HTTP Clients
MIT License
480 stars 147 forks source link

Run tests against different Rails versions #35

Closed awendt closed 10 years ago

awendt commented 10 years ago

This introduces the use of Appraisal to test against Rails 3.1, 3.2, and 4.

It fixes a number of issues I had when running tests against those versions.

I just discovered #34 and realized I'd effectively dropped support for Ruby 1.8 :grimacing: I guess this just got on the top of my TODO list.

To run against those Rails versions locally:

  1. rake appraisal:install
  2. rake appraisal spec

I'd love to see this merged, but I'm still eager to get feedback on it.

Also note that this will run all specs (even the ones not specific to Rails) against different Rails versions. I figured that the value of having specs run against Rails versions that have been released after 2009 outweighs the harm of needlessly re-running other specs.

Umofomia commented 10 years ago

This is great! Being that the gemspec still currently specifies "~> 2.3.2" for activesupport and activeresource, should this also be made to work with Rails 2.3 (despite the fact that official support for it has lapsed)? I still unfortunately have to deal with Rails 2.3 projects and do make use of this gem in them.

RSpec 2 doesn't really play well with Rails 2.3 though, but it is possible to make Appraisal switch to using RSpec 1 for Rails 2.3; I have done this in other gems in the past. Let me know what you think and whether you need any assistance with this.

awendt commented 10 years ago

@Umofomia In the current release, this gem is tested with RSpec 2 against Rails 2.3, so I guess it works out okay.

Anyway, seeing that there's a need to continue support Ruby 1.8 and Rails 2.3, I propose going forward with Rails 3 and 4 with master toward 2.x while keeping a stable branch for Rails 2.3 on Ruby 1.8.

@mgomes What do you think?

mgomes commented 10 years ago

@awendt I support to move to Ruby 1.9+ and Rails 3.2+. If we only support Rails 3.2+, should we remove Activeresource as a client?

kjg commented 10 years ago

@awendt What would it take to get this working with Rails 2.3 and ruby 1.8? I'd love to get this merged in as well.

awendt commented 10 years ago

@kjg We can't get this to work with Rails 2.3 on Ruby 1.8 and 3.2 (and later) on 1.9 on a single branch. This is because ActionController::Request is no longer available in Rails. If we did this in a single branch, the code gets really ugly...

I suggest:

  1. branching off "stable-v1" (or whatever name suits best)
  2. then merge this into current master towards a 2.0 release
  3. re-enable tests for Ruby 2.0 and 2.1

If you want, I can work on another PR that uses Appraisal for Rails 2.3 on the stable-v1 branch, but since we not really need to test against several versions, Appraisal would be overkill for that branch.

Umofomia commented 10 years ago

@awendt What's currently preventing it from working? It currently appears that only the specs refer to ActionController::Request directly. The actual gem only refers to it as a string so it should still work. The spec issue should be able to be easily worked around.

awendt commented 10 years ago

@Umofomia You're absolutely right about ActionDispatch vs. ActionController. In addition to that, most of my incompatible changes between 2.3/3.0 and 3.1, 3.2, and 4.0 are in 62c6fdfd673eefaa3ffed6f751cd2e857344f13a (railtie spec):

fails because of routing problems in 3.0 and because of NoMethodError on action in 2.3. (This is also a test-only problem.)

Frankly, I don't see the value in bloating the code base (and tests) with version checking of Rails (which you might not even use in addition to this gem), only for the sake of testing api_auth against the last 5 major versions of Rails instead of the last 3 (not even counting the latest, 4.1).

Please note I'm not proposing to ditch support for Rails 2.3. I'm merely saying it's time for the next release of this gem to move tests into this decade without bloat.