3
\$\begingroup\$

I have the following Ruby code in an RSpec file:

  describe "order" do
    before do
      LIST_LENGTH ||= 10
      @skills = FactoryGirl.create_list(:skill, LIST_LENGTH)
      @developer = FactoryGirl.create(:user)
      @requests = FactoryGirl.create_list(:request, LIST_LENGTH)
      LIST_LENGTH.times { |i| @requests[i].skills = @skills[0..i] }
    end 

    describe "#order_by_interestingness_for" do
      it "orders by interestingness" do
        @developer.add_interested_skills(@skills)
        Request.all.shuffle.map {
          |r| r.interestingness_for(@developer)
        }.should_not be_in_order

        Request.order_by_interestingness_for(@developer).map {
          |r| r.interestingness_for(@developer)
        }.should be_in_order
      end 
    end 

    describe "#order_by_qualifiedness_for" do
      it "orders by interestingness" do
        @developer.add_proficient_skills(@skills)
        Request.all.shuffle.map {
          |r| r.qualifiedness_for(@developer)
        }.should_not be_in_order

        Request.order(:interestingness, :developer => @developer).map {
          |r| r.qualifiedness_for(@developer)
        }.should be_in_order
      end 
    end 
  end 

Obviously, this is not very DRY. I don't really know how to make it more DRY, though, since the the unique parts are so deeply ingrained in the code. I'm guessing I have a deeper design problem. I'm not really sure, though. Any ideas?

\$\endgroup\$
  • 2
    \$\begingroup\$ I'm unsure of what you're accomplishing with the first repeated bit; you call Request.all.shuffle, so yes, the resulting list should not be in order. Ruby's code has been tested, so no reason for you to do it. Moreover, shuffle is random, so the shuffled list may in fact be in order, and your test will randomly fail. But fail or pass, what does it prove exactly? \$\endgroup\$ – Flambino Oct 13 '12 at 8:49
2
\$\begingroup\$

Some suggestions:

  • There's no need for the negative assertion. You're testing Ruby more than you're testing your own code and in the end, we don't actually care. It only matters that our attempt to order the results behaves as expected. Assuming they're un-ordered otherwise (or at least order isn't guaranteed).
  • Favor RSpec's 'let' over instance variables and constants in your tests.
  • RSpec's 'should' syntax will be deprecated in the next release. Favor using the new 'expect' syntax instead.
  • Use string literals when you can.
  • Use a shared example to DRY up your tests a bit.
  • Modify the underlying implementation and the public API in your code for better symmetry between ordering by 'interestingness' and 'qualifiedness'. I suspect there's some code duplication in there given the current API that could also be eliminated.

Do all that, and you could end up with a test that looks something like this:

shared_examples_for 'a ordered request' do |order_type, dev|
  it "orders by #{order_type}" do
    result = Request.call(:order, { order_type, :developer => dev })
    result.map! { |r| r.send("#{order_type}_for", dev) }
    expect(result).to be_in_order
  end
end

describe Request do
  let(:list_length) { 10 }
  let(:skills) { FactoryGirl.create_list(:skill, list_length) }
  let(:developer) { FactoryGirl.create(:user) }
  let(:requests) { FactoryGirl.create_list(:request, list_length) }

  before { list_length.times { |i| requests[i].skills = skills[0..i] } }

  describe '#order' do
    context 'when ordering by interestingness'
      before { developer.add_interested_skills(skills) }
      it_behaves_like 'a ordered request', 'interestingness', developer
    do

    context 'when ordering by qualifiedness' do
      before { developer.add_proficient_skills(skills) }
      it_behaves_like 'a ordered request', 'qualifiedness', developer
    end
  end
end

I'd also recommend checking out this site which is a pretty good, but still growing style guide for writing behavior tests in RSpec.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.