Add more Spec command test coverage #1675

Closed
wants to merge 4 commits into
from

Projects

None yet

3 participants

@carsonmcdonald
Contributor

More coverage and a minor bug fix.

@fabiopelosin
Member

Great work! Please let me know when you are finished with the patch so I can review and merge it!

@carsonmcdonald
Contributor

@irrationalfab The intention here was to cover choose_from_array and fix a small bug. I believe that is done but let me know if you think it needs more.

@alloy alloy commented on the diff Dec 13, 2013
spec/functional/command/spec_spec.rb
run_command('spec', 'cat', '--show-all', 'AFNetworking')
UI.output.should.include fixture('spec-repos/master/AFNetworking/1.2.0/AFNetworking.podspec').read
+ $stdin = STDIN
@alloy
alloy Dec 13, 2013 Member

This will fail to execute if one of the assertions fails, because a failed assertion raises an exception. We will need a better SpecHelper module for this that does something like:

module SpecHelper::OutputCapture
  def it(*args, &block)
    $stdin = StringIO.new("1\n", 'r')
    super
  ensure
    $stdin = STDIN
  end
end

describe "output capturing" do
  extend SpecHelper::OutputCapture

  it "cats the first podspec from all podspecs" do
    run_command('spec', 'cat', '--show-all', 'AFNetworking')
    UI.output.should.include fixture('spec-repos/master/AFNetworking/1.2.0/AFNetworking.podspec').read
  end
end
@carsonmcdonald
carsonmcdonald Dec 13, 2013 Contributor

Good catch. I will follow up with something to wrap it. In doing this I was wondering if the interaction with stdin should be rolled into UI?

@fabiopelosin
fabiopelosin Dec 13, 2013 Member

I like the idea of using UI.

@alloy
alloy Dec 13, 2013 Member

On that note, it might be better if we can assign an IO instance on the UI module and set that to a StringIO for the duration of such a capture block. This way we also ensure that someone doesn’t accidentally use plain puts.

@fabiopelosin fabiopelosin added a commit that closed this pull request Dec 13, 2013
@carsonmcdonald @fabiopelosin carsonmcdonald + fabiopelosin Add more Spec command test coverage
Change STDIN to $stdin so it can be reassigned
Use UI.puts instead of print
Add test coverage for Spec#choose_from_array
Fix off by one issue in Spec#choose_from_array

Closes #1675
b68d61d
@fabiopelosin
Member

@alloy Sorry I totally missed your comment! 😶

@carsonmcdonald Firs of all, thanks for the contribution 🍻! I asked because I merged the other similar pull request and I was wondering if at the time you where planning to expand it.

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