Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Brittle ActiveRecord tests #71

Closed
soulcutter opened this Issue Jan 24, 2013 · 4 comments

Comments

Projects
None yet
3 participants
Contributor

soulcutter commented Jan 24, 2013

Working on some validation specs I noticed that they're fairly brittle and require manual attention to generate sqlite3 db's (which actually live in the repo).

I propose that the sqlite3 files be removed from the repo at a minimum - either being generated on-demand by the spec task, or removing the whole dummy application entirely in favor of testing validations using something more like:

require 'spec_helper'

describe MoneyRails::ActiveModel::MoneyValidator do
  let(:model_class) do
    Struct.new(:price_cents, :price_currency) do
      include ActiveModel::Validations
      include ActiveModel::Validations::Callbacks
      include ActiveModel::Dirty

      include MoneyRails::ActiveRecord::Monetizable

      ## ActiveModel::Dirty stuff
      define_attribute_methods [:price_cents]

      def price_cents=(price)
        price_cents_will_change! unless price == @price_cents
        @price_cents = price
      end
    end
  end

  subject(:model) { validation_class.new }

  before(:each) do
    stub_const('BaseModel', model_class)
    stub_const('ExampleModel', validation_class)
  end

  context "default validation" do
    let(:validation_class) do
      Class.new(model_class) do
        monetize :price_cents
      end
    end

    it "validates numericality" do
      model.price = "asdfasdf"
      model.should_not be_valid
    end
  end
end

The downside is that it doesn't look as much like it would actually be used, but it doesn't require a db and is far simpler to maintain than a whole rails app. Another downside is that the mongoid integration would need to get pulled out as well to be able to get rid of the whole app.

Would love to hear some feedback before such a potentially large refactor.

Contributor

soulcutter commented Jan 25, 2013

soulcutter/money-rails@c67c5cd

^^ This could still use some work, but what do you think? I commented out a handful of specs that I thought were questionable for various reasons. Also I think I exposed a problem with some assignments that don't trigger dirty attributes, leading to validation passing where it should fail - fixed here

Owner

alup commented Jan 27, 2013

@soulcutter I agree with your proposal :)! But I would prefer a step by step migration towards this direction. I would propose as a first step, to generate the repo on-demand by the spec tasks and as a second step (if we have a reason to do it) to fully decouple the tests from the dummy application. What do you think?

P.s. thanks for your time!

Owner

alup commented Jan 27, 2013

Another idea that would affect our decision, is the need to test multiple rails versions. I was thinking to use "appraisal" for this purpose.

Owner

semmons99 commented Nov 28, 2013

no activity, closing

@semmons99 semmons99 closed this Nov 28, 2013

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