JEG2 / highline

A higher level command-line oriented interface.
Other
1.29k stars 137 forks source link

Can't test expected output when using say #176

Closed alem0lars closed 9 years ago

alem0lars commented 9 years ago

I'm writing a spec that tests a method successfully outputs some strings. However, inside that method I use say instead of the builtin puts. This leads to no output catched by rspec.

Example:

def log_msg(msg)
  say msg
end
context "when log a message" do
  subject { -> { log_msg("foo") } }
  it { is_expected.to output("foo\n").to_stdout }
end

The spec above doesn't pass:

...
expected block to output "foo\n" to stdout, but output nothing
...

If I replace say with puts it will pass.

alem0lars commented 9 years ago

Also, testing user input doesn't work either.

I try to intercept user input using:

before { $stdin = StringIO.new("y") }

but highline still prompts for user input and ignores the $stdin.

abinoam commented 9 years ago

For the second comment, HighLine saves what $stdin is pointing to at the time you require 'highline/import' or instantiate the HighLine intance. Perhaps RSpec is not having time to redirect it.

Try to something like

before do
  input = StringIO.new("y")
  cli = HighLine.new(input)
end

describe "something" do
  it "should work" do
    cli.ask "Yes or no"
  end
end

This way you don't need to require 'highline/import'. Only 'highline'.

alem0lars commented 9 years ago

Unfortunately that's not my usecase @abinoam .

I don't do dependency injection (e.g. there is no cli argument). It's directly called inside methods (as usual printings).

abinoam commented 9 years ago

Hi @alem0lars,

From the RSpec output matcher documentation at https://www.relishapp.com/rspec/rspec-expectations/docs/built-in-matchers/output-matcher

Note: to_stdout and to_stderr work by temporarily replacing $stdout or $stderr, so
they're not able to intercept stream output that explicitly uses STDOUT/STDERR or that
uses a reference to $stdout/$stderr that was stored before the matcher is used.

So, when you do require 'highline/import' at your code (or at your test code) you're saving the reference to whatever $stdin and $stdout is pointing to at that moment. So HighLine doesn't get affected by the later RSpec stdout/stdin redirection.

I think I can help you to work around the issue. If you don't have a way to do as I explained in my previous comment, then let's try a more convoluted way, that may help you.

When requiring 'highline/import' HighLine saves a HighLine instance to be used as something like a default HighLine instance for #say, #ask and others at the global variable $terminal (although this behaviour may be deprecated soon)..

So you can directly set the input and output buffer at rspec time to whatever you want. The instance variables are @input and @output.

$terminal.instance_variable_set(:@input, STDIN)
$terminal.instance_variable_set(:@output, STDOUT)

Just change STDIN and STDOUT at the example to whatever you need.

And, please gives a feedback if it works.

abinoam commented 9 years ago

If you need, have a look at https://github.com/JEG2/highline/blob/master/test/test_highline.rb to see how we test for input and output.

alem0lars commented 9 years ago

Yes thanks to the two comments above, I've been able to create specs using highline.

Just for future reference, I leave here the shared_context I've created:

shared_context :highline do
  def provide_input(text)
    @input << "#{text}\n"
    @input.rewind
  end

  def consume_output
    text = @output.string.dup
    @output.truncate @output.rewind
    text
  end

  def reset_terminal(input, output)
    @input  = input
    @output = output
    $terminal.instance_variable_set :@input,  @input
    $terminal.instance_variable_set :@output, @output
  end

  before { reset_terminal StringIO.new, StringIO.new }
  after  { reset_terminal STDIN,        STDOUT       }
end

This allows you to easily create specs using highline.


@abinoam , @JEG2 Maybe we should add that snippet in the readme ?

alem0lars commented 9 years ago

Another (more concise and betterspecs compliant) version:


shared_context :highline do
  let!(:input) { StringIO.new }
  before { $terminal.instance_variable_set :@input, input }

  let!(:output) { StringIO.new }
  before { $terminal.instance_variable_set :@output, output }

  def provide_input(text)
    input << "#{text}\n"
    input.rewind
  end

  def consume_output
    text = output.string.dup
    output.truncate output.rewind
    text
  end
end
abinoam commented 9 years ago

I'm happy that you have made it. Thank you for reporting back so other may benefit!

About the README, I don't completely agree. We're at a discussion for changing the api. We think that we shouldn't contaminate the main namespace by default anymore. The main way we should recommend will be:

require 'highline'     # not 'highline/import
cli = HighLine.new
cli.say "Anything you want to say

It's already on the README of the master branch. So your example will not be functional.

But, I'm really happy that you have reported it in such detailed way. If anybody cross this problem later, we can easily point him/her to your solution.