1

Given this code:

field :start_now, type: Boolean, :default => true
field :time_zone
validates :time_zone, inclusion: { 
              in: ActiveSupport::TimeZone.zones_map.keys 
           }, unless: :start_now?

I have created this rspec test, but it's long and not DRY. And the only reason to be like this is because of the "unless" condition:

describe "#time_zone" do
  context "scheduled" do
    before :each do
       subject.start_now = false
    end
    it { is_expected.to validate_inclusion_of(:time_zone).in_array(ActiveSupport::TimeZone.zones_map.keys) }
  end
  context "run now" do
    before :each do
       subject.start_now = true
    end
    it { is_expected.not_to validate_inclusion_of(:time_zone).in_array(ActiveSupport::TimeZone.zones_map.keys) }
  end
end

Is there any shorter way to do this?

Fran Martinez
  • 2,994
  • 2
  • 26
  • 38

3 Answers3

1

There's really no need to make it any shorter than this. Both tests are describing exactly what they're testing, and they're very specific about those tests.

From what I've seen with testing, you want your tests to be DAMP. This doesn't mean that they're not DRY, but it does mean that you shouldn't bend over backwards to make it as small and narrow as possible.

Your code has two distinct and mutually exclusive states, which depends on the boolean start_now?. You have to test both states, and there's really no way around that. There's no repetition either; you're testing two distinct branches.

While your contexts could use a wee bit of a verbiage clean up, they're pretty good at describing what it is you need to do.

Community
  • 1
  • 1
Makoto
  • 104,088
  • 27
  • 192
  • 230
1

Here and improvement. Next code is more DRY and at the same time more DAMP as before:

describe "#time_zone" do    
  let(:validate_time_zone) { validate_inclusion_of(:time_zone).in_array(ActiveSupport::TimeZone.zones_map.keys }

  context "scheduled" do
    before :each do
       subject.start_now = false
    end
    it { is_expected.to validate_time_zone }
  end
  context "run now" do
    before :each do
       subject.start_now = true
    end
    it { is_expected.not_to validate_time_zone}
  end
end

Ok, so I've understand that to be DAMP (a new word for me) is good. I feel better today because I was always thinking about the problems of being too much DRY with tests and making them very difficult to read.

Fran Martinez
  • 2,994
  • 2
  • 26
  • 38
1

Sometimes I like to write my specs using the let inside of each context that is called in the external before :each. This avoids repeating the before :each for each context.

describe "#time_zone" do    
  let(:validate_time_zone) do
    validate_inclusion_of(:time_zone).in_array(ActiveSupport::TimeZone.zones_map.keys
  end

  before :each do
    subject.start_now = start_now
  end

  context "scheduled" do
    let(:start_now) { false }
    it { is_expected.to validate_time_zone }
  end

  context "run now" do
    let(:start_now) { true }
    it { is_expected.not_to validate_time_zone}
  end
end