search

jrgifford / delayed_paperclip

Process your Paperclip attachments in the background with delayed_job or Resque.
http://www.jstorimer.com/ruby/2010/01/30/delayed-paperclip.html
MIT License
402 stars 155 forks source link

Refactoring/Ading specs #47

Closed ScotterC closed 11 years ago

ScotterC commented 11 years ago

I started off by thinking I'd fix the Paperclip 3.5 issues. Then I realized that the test suite has gotten a bit unwieldy and the entire library is super confusing in respect to how small it is. I decided real unit tests were in order so I wrote them in rspec because it's a lot easier to modularize the pertaining logic. All the old tests still pass as well as the rspec ones.

ScotterC commented 11 years ago

@jrgifford I can't figure out how to get Travis to listen to the appraisal gemfiles. Have you seen this build error before?

jrgifford commented 11 years ago

I don't think I have. I'll take a stab at it and see what I can come up with though.

ScotterC commented 11 years ago

I talked to the Travis guys. Apparently it has to do with the Gemfile.lock's path. It needs to be a relevant path instead of hard set. Bug in bundler that's fixed but not released yet. https://github.com/bundler/bundler/blob/master/CHANGELOG.md#136

jrgifford commented 11 years ago

Ok. Question that might be worthy of a separate bug report: Do we need ruby- in the .ruby-version file? Or does that break things in RVM? rbenv complains about it...

ScotterC commented 11 years ago

interesting. The reason I was doing it because I thought it was what rbenv and rvm agreed on these days. With rvm installed it recommends the change. Happy to revert it.

jrgifford commented 11 years ago

The current contents of .ruby-version is:

ruby-2.0.0-p195

rbenv complains with the following:

$ ruby -v
warning: ignoring extraneous `ruby-' prefix in version `ruby-2.0.0-p195'
         (set by /home/jrg/code/jrgifford/delayed_paperclip/.ruby-version)
ruby 2.0.0p195 (2013-05-14 revision 40734) [x86_64-linux]

If it changes to just the version number

2.0.0-p195

it doesn't say anything.

Is the ruby- prefix necessary? I don't have a rvm system around, otherwise I'd test.

(isn't that important, it's just semantics. But it's a semantics that I'm unsure of how to proceed with.)

mpapis commented 11 years ago

2.0.0-p195 will be fine for all rvm, rbenv and chruby, the problem is this name can be ambiguous and future versions of rubinius could be confused with ruby in case of the 2.0.0 version

ScotterC commented 11 years ago

Alrighty. Added all the integration tests. Covering as much as they did and it's a lot easier to read. Found a lot of duplicate testing etc.

Found that the ruby- wasn't necessary for the ruby-version file which is good.

@mpapis thanks for the note the rake build. Glad to see that test unit and rspec don't clash.

ScotterC commented 11 years ago

What's driving me nuts right now is the specs are passing but I can't figure out how to silence the warnings on Travis. I'm not getting them locally. Any ideas?

EDIT:
Nevermind. Got it

ScotterC commented 11 years ago

@jrgifford do you have any allegiance to the Test::Unit suite? Because I've fully replicated it now and would be happy to remove it completely at this point.