CODEWITHSUNDEEP
rubyrspec
rubyist
rubyist

Reputation: 3132

refactor the rspec tests

I have written a unit tests as below.

describe Youtube::Metadata do
  let(:subject) { SampleMetadata.from_text(open(url)) }
  context "when url is passed" do
    let(:url) { "http://www.sample.com" }

    describe "It should return all values from the site" do
      its(:id) { should eql "234" }
      its(:url) { should eql "www.sample.com" }
    end

    context "Given a html file with id property" do
      let(:html) { File.join(PAGES, 'youtube', 'sample1.html') }
      let(:subject) { Youtube::Metadata.from_text(File.read(html)) }

      it "should return unique value from id property" do
        subject.unique_id.should eql "65155c6e-ba11-42fc-bb91-53520176d2a8"
      end
    end

    context "Given a html file with rel:canonical property" do
      let(:html) { File.join(PAGES, 'youtube', 'sample2.html') }
      let(:subject) { Youtube::Metadata.from_text(File.read(html)) }

      it "should return unique value from rel=canonical content property" do
        subject.unique_id.should eql "65155c6e-ba11-42fc-bb91-53520176d2a8"
      end
    end
  end
end

I need to refactor it so that I dont have to repeat the below statement in each and every tests cases.

let(:subject)   { Youtube::Metadata.from_text(File.read(html))

I tried it with before block but it didn't work.

Actually the first context we pass url and the second set of context we pass filename. Also I am not sure whether I need to make it as a single context (which is passing file name contexts, ie. last two) and describe it.

What would be the right way to do it??

Upvotes: 0

Views: 125

Answers (4)

Anthony
Anthony

Reputation: 15957

I think you're asking good questions but let's take a step back regarding testing in general. You're looking to DRY up your tests because you recognize duplicate code - great. Tests are meant to be easy to read and easy to follow.

Thankfully rspec gives you access to things like context and describe in order for you to separate your tests into parts. Your tests are heavily separated and you're using let variables (which in turn create instance variable scopes) for things you're only using in a single test.

To me that seems overkill and you're adding complexity to a test that is otherwise self contained in a normal it block.

For instance, this seems like a more structured set of tests:

describe Youtube::Metadata do
  include Helpers
    # ... omitted ... 

    context "Given a html file with id property" do
      it "should return unique value from id property" do
        subject = file_loader('youtube','sample1.html')
        subject.unique_id.should eql "65155c6e-ba11-42fc-bb91-53520176d2a8"
      end
    end

    context "Given a html file with rel:canonical property" do
      it "should return unique value from rel=canonical content property" do
        subject = file_loader('youtube','sample2.html')
        subject.unique_id.should eql "65155c6e-ba11-42fc-bb91-53520176d2a8"
      end
    end
  end
end

Then just create a helper like so:

module Helpers
  def file_loader(dir, filename)
    file = File.join(PAGES, dir, filename)
    Youtube::Metadata.from_test(file)
  end
end

Upvotes: 2

Uri Agassi
Uri Agassi

Reputation: 37409

You can use shared_examples:

shared_examples_for "unique id parser" do |sample_file, expected_id|
  let(:html) { File.join(PAGES, 'youtube', sample_file) }
  subject { Youtube::Metadata.from_text(File.read(html)) }

  it "parses unique id property" do
    expect(subject.unique_id).to eql expected_id
  end
end

context "Given a html file with id property" do
  it_behaves_like "unique id parser", 'sample1.html', "65155c6e-ba11-42fc-bb91-53520176d2a8"
end

context "Given a html file with rel:canonical property" do
  it_behaves_like "unique id parser", 'sample2.html', "65155c6e-ba11-42fc-bb91-53520176d2a8"
end

Upvotes: 1

Stefan
Stefan

Reputation: 114138

Group it with another context:

describe Youtube::Metadata do
  # ...
  context "given a html file" do
    subject { Youtube::Metadata.from_text(File.read(file)) }

    context "with id property" do
      let(:file) { File.join(PAGES, 'youtube', 'sample1.html') }

      it "should return unique value from id property" do
        # ...
      end
    end

    context "with rel:canonical property" do
      let(:file) { File.join(PAGES, 'youtube', 'sample2.html') }

      it "should return unique value from rel=canonical content property" do
        # ...
      end
    end
  end
end

Upvotes: 0

David V. Teixeira
David V. Teixeira

Reputation: 57

First, use another name for the second and third ":subject" because you already has one in the outter block.

Second, you really need 'sample1.html' and 'sample2.html'? If not, change to 'sample.html' and move both 'let's up.

Third, check if you had some problem with lazzy load (change let to let!)

Upvotes: 0

Related Questions