Rspec-core: What is RSpec contributors' attitude towards dynamic specs?

[marko-avlijas]

I apologize if this is wrong place to ask (what would be better place)?

Here is an example from https://coderwall.com/p/5kfxhg/dynamic-rspec-tests

describe AmbiguousTime do
  times = { 43200 => '12:00PM',
            72000 => '8:00PM',
            84600 => '23:30PM',
            27000 => '7:30AM',
            55800 => '3:30PM',
            55800 => '3:30'  }

  times.each do |planned_output, input|
    it "should return #{planned_output} from #{input}" do
      actual_output = AmbiguousTime::convert(input)
      actual_output.should eql(planned_output)
    end
  end
end

Is this considered good practice or bad? And why?

And what about using it to create a bunch of really detailed specs and then repeating them.
2 cases come to mind:
1) What if I am testing a module which will be included in several classes?
Should I create a detailed shared example specs for module and include it in every class?

2) What about functional tests?
If I create detailed unit tests for low level, should I extract them into shared examples and also duplicate them in mid level class that is using low level class?
I always find advice to write less functional tests, but what's wrong with repeating them in dry way (by including shared examples?)

[penelopezone]

@marko-avlijas thanks for filing this, but really, the RSpec mailing list is the best place to ask questions like this. You should head over to https://groups.google.com/forum/#!forum/rspec and ask there.

[JonRowe]

Is this considered good practice or bad? And why?

It's fine to do that, but you might consider making it one spec like this:

RSpec.describe "AmbiguousTime" do
  it "returns the correct planned output" do
    expect({
      43200 => '12:00PM',
      72000 => '8:00PM',
      84600 => '23:30PM',
      27000 => '7:30AM',
      55800 => '3:30PM',
      55800 => '3:30'
    }).to all { |input, output| AmbiguousTime::convert(input) == output }
  end
end

1. What if I am testing a module which will be included in several classes? Should I create a detailed shared example specs for module and include it in every class?

Theres two approach you can use, you can either create a shared example set and include them in every class as you mention, or you can test the module once on an anonymous class. The later is more unit like.

1. What about functional tests?
   If I create detailed unit tests for low level, should I extract them into shared examples and also duplicate them in mid level class that is using low level class?

Generally if it's covered by unit tests you don't need to duplicate those tests at a higher level, your higher level tests should cover the interconnectivity of units and how they might interact.

I always find advice to write less functional tests, but what's wrong with repeating them in dry way (by including shared examples?)

"Rails" functional tests usually carry this advice because it's slow. Repeating them when you've covered those edge cases elsewhere adds to your build time without adding value.

[marko-avlijas]

Thank you

[marko-avlijas]

@JonRowe - I couldn't get your example to work. I'd always get an unhelpful message saying:
ArgumentError: wrong number of arguments (given 0, expected 1)

It seems that all matcher must be called with a parameter and only way I could make this work was by doing something like:

RSpec.describe "AmbiguousTime" do
  it "returns the correct planned output" do
    {
      43200 => '12:00PM',
      72000 => '8:00PM',
      84600 => '23:30PM',
      27000 => '7:30AM',
      55800 => '3:30PM',
      55800 => '3:30'
    }.each do |input, output| 
      expect(AmbiguousTime::convert(input)).to eq(output)
    end
  end
end

[JonRowe]

You possibly have capybara interfering, I often do RSpec.alias_matcher :all, :satisfy to get around this, making the last line }).to satisfy { |input, output| AmbiguousTime::convert(input) == output }

[myronmarston]

You possibly have capybara interfering

I believe it's just our built-in all matcher -- it requires an argument and does not expect a block:

https://github.com/rspec/rspec-expectations/blob/v3.5.0/lib/rspec/matchers.rb#L651-L668

...so I would absolutely expect users to get the kind of error @marko-avlijas mentioned.

I often do RSpec.alias_matcher :all, :satisfy to get around this

I wouldn't recommend this, as the all matcher is quite useful in its own right, and aliasing all to do something else will cause confusion and prevent you from having access to the useful functionality all provides.

If you want to use all here, you need to pass a matcher; the simplest one is probably satisfy since satisfy is essentially an adapter that takes any block and adapts it to the matcher protocol:

RSpec.describe "AmbiguousTime" do
  it "returns the correct planned output" do
    expect({
      43200 => '12:00PM',
      72000 => '8:00PM',
      84600 => '23:30PM',
      27000 => '7:30AM',
      55800 => '3:30PM',
      55800 => '3:30'
    }).to all satisfy { |(input, output)| AmbiguousTime::convert(input) == output }
  end
end

(Note, I haven't tried that, but I believe it should work).

Personally, I'd probably use aggregate_failures for this:

  aggregate_failures do
    {
      43200 => '12:00PM',
      72000 => '8:00PM',
      84600 => '23:30PM',
      27000 => '7:30AM',
      55800 => '3:30PM',
      55800 => '3:30'
    }.each do |input, output| 
      expect(AmbiguousTime::convert(input)).to eq(output)
    end
  end

...which will run the block to completion and then give you a list of all failures instead of just one.

[JoshCheek]

Taking it fairly verbatim, I'd write it like this:

AmbiguousTime = Module.new
def AmbiguousTime.convert(seconds)
  /^(\d+):(\d+)(AM|PM)?$/ =~ seconds
  hours, minutes = $1.to_i, $2.to_i
  hours += 12 if $3 == 'PM' && hours < 12
  hours*60*60 + minutes*60
end

require 'rspec/autorun'
RSpec.configure { |c| c.fail_fast, c.formatter = true, 'documentation' }
RSpec.describe 'AmbiguousTime' do
  [ ['12:00PM', 43200],
    ['8:00PM',  72000],
    ['23:30PM', 84600],
    ['7:30AM',  27000],
    ['3:30PM',  55800],
    ['3:30',    12600],
  ].each do |input, output|
    example "#{described_class}.convert(#{input.inspect}) == #{output.inspect}" do
      expect(AmbiguousTime.convert input).to eq output
    end
  end
end

# >> 
# >> AmbiguousTime
# >>   AmbiguousTime.convert("12:00PM") == 43200
# >>   AmbiguousTime.convert("8:00PM") == 72000
# >>   AmbiguousTime.convert("23:30PM") == 84600
# >>   AmbiguousTime.convert("7:30AM") == 27000
# >>   AmbiguousTime.convert("3:30PM") == 55800
# >>   AmbiguousTime.convert("3:30") == 12600
# >> 
# >> Finished in 0.00519 seconds (files took 0.1203 seconds to load)
# >> 6 examples, 0 failures
# >> 

Or maybe like this:

RSpec.describe 'AmbiguousTime' do
  def self.converts!(input, output)
    example "#{described_class}.convert(#{input.inspect}) == #{output.inspect}" do
      expect(AmbiguousTime.convert input).to eq output
    end
  end
  converts! '12:00PM', 43200
  converts! '8:00PM',  72000
  converts! '23:30PM', 84600
  converts! '7:30AM',  27000
  converts! '3:30PM',  55800
  converts! '3:30',    12600
end

• I usually quote the name of the class because I often write the spec sufficiently far in advance that the class doesn't exist yet, and Ruby will explode before RSpec starts running if the constant isn't defined. At the same time, there is value in the alternative, because we can use described_class, which decouples tests from implementation details. I still like my way better, if it's an issue, I make a helper method to encapsulate invocation (the converts! in the second example).
• When I make helper methods to encapsulate the invocation, I often suffix them with a bang. This is my own idiom, I use it in tests to imply that the method is an assertion (I just get tired of writing the assert_ prefix, here are some examples in the wild)
• I use example here over it because there is no description of behaviour, it's purely a set of inputs and outputs, and I must guess the reason why it maps the way it does. This is my primary criticism of the test, it does not describe behaviour, and I think that describing behaviour is incredibly valuable (because it forces us to consider ontology).
• I got rid of "should" in part because it's spam and in part because "should" implies a "test-after" paradigm. Assert that the world is the way you want it to be, not that it should be the way you want it to be. _"When you turn the steering wheel to the left, the car should turn left"_ why introduce doubt here? It does or it's wrong.
• Personally, I would prefer that tests declared with example not show up in the output except when they fail, because I set the output format to documentation, and examples are just spam in the output. For that reason, I very well might move the entire loop into the test. I often consolidate tests when separating them would spam the output. The downside is that there's more ways to fail and if we're not attentive, we can get mixed up about what is failing. There are tricks to mitigate this, I'm sure many people would chastise me if they saw me use them. Nonetheless, my experience leads me to believe that it's frequently a better balance.
• I swapped the order of the key/value pairs because I think it makes more sense to map an input to an output.
• I went with an array instead of a hash because the format of the data does not imply unique keys to me, eg in the original example, "3:30PM" and "3:30" both parse to the same value. Note that this means the two keys overwrite each other and thus one of them does not run. Ie 5 specs will run where there should have been 6, because the second one with key 55800 wiped out the first.
• I assume that "3:30" should convert to 12600, even though the spec says it should parse to 55800. Maybe it was supposed to imply that an omitted meridian implies PM, but I didn't see any reason for that to be the case (b/c that would be "15:30"), so I assumed that the test was incorrect. Further evidence of this was that the key overrode the key for "3:30PM", so there is already one error in the test on this value, and it's the last value and the only one without a meridian, so it makes more sense to me that it was written while tired of entering examples and wasn't written very thoughtfully. Notice how I'm having to use contextual clues, this is a common downside of example based testing.

---

I think it's shitty to describe a thing without showing it since showing it makes it concrete (in this case, behaviour specification over example based testing). So I rewrote it to illustrate what I think is better. Surprisingly enlightening as I wound up discovering errors in my implementation. Additionally, I thought of several tests that weren't provided, but decided to omit them because it wasn't clear to me what the behaviour should be.

AmbiguousTime = Module.new
def AmbiguousTime.convert(seconds)
  /^(\d+):(\d+)(AM|PM)?$/ =~ seconds
  hours, minutes = $1.to_i, $2.to_i
  hours %= 12 if $3 != 'PM' && hours < 13
  hours += 12 if $3 == 'PM' && hours < 12
  hours*60*60 + minutes*60
end

require 'rspec/autorun'
RSpec.configure { |c| c.fail_fast, c.formatter = true, 'documentation' }
RSpec.describe 'AmbiguousTime' do
  def convert!(input, output)
    expect(AmbiguousTime.convert input).to eq output
  end
  it 'converts HH:MMa to seconds' do
    convert! '1:23AM',      1*60*60 + 23*60
    convert! '1:23PM', (12+1)*60*60 + 23*60
  end
  it 'starts each day at 12:00AM' do
    convert! '12:00AM', 0
  end
  it 'wraps time at 12:00PM' do
    convert! '12:00PM', 12*60*60
  end
  it 'considers times without meridians to be based on a 24 hour clock' do
    convert!  '1:23',      1*60*60 + 23*60                                 # => true
    convert! '13:23', (12+1)*60*60 + 23*60
  end
  it 'ignores the meridian on 24 hour clock inputs' do
    convert! '13:23AM', (12+1)*60*60 + 23*60
    convert! '13:23PM', (12+1)*60*60 + 23*60
  end
end

# >> 
# >> AmbiguousTime
# >>   converts HH:MMa to seconds
# >>   starts each day at 12:00AM
# >>   wraps time at 12:00PM
# >>   considers times without meridians to be based on a 24 hour clock
# >>   ignores the meridian on 24 hour clock inputs
# >> 
# >> Finished in 0.00493 seconds (files took 0.19827 seconds to load)
# >> 5 examples, 0 failures
# >>