Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

About output_stream setting #1235

Closed
untidy-hair opened this issue Dec 30, 2013 · 5 comments
Closed

About output_stream setting #1235

untidy-hair opened this issue Dec 30, 2013 · 5 comments

Comments

@untidy-hair
Copy link

Hi, I had an small issue with the combination of rspec + spork (and have a workaround) as on here:
sporkrb/spork#248

I saw the changes about "output_stream" in rspec-core and also read the discussions like #1127 , #1128 , #1142.

The original spork code is:

::RSpec::Core::CommandLine.new(argv).run(stderr, stdout)

It doesn't work well because $stdout value changes between when 'spork'ing and when 'rspec'ing, so my workaround is:

configuration = ::RSpec::configuration
configuration.output_stream = stdout
::RSpec::Core::CommandLine.new(argv, configuration).run(stderr, stdout)

I understand now we should set up io streams with RSpec::Core::Configuration and I think that's a great idea.

But maybe we should make Core::CommandLine#run arguments options(or remove?), too, so that we don't have to repeat specifying 'stdout' above?

Thanks!

@myronmarston
Copy link
Member

Thanks for reporting this!

It's unclear to me why ::RSpec::Core::CommandLine.new(argv).run(stderr, stdout) is failing; my change in #1142 was designed to preserve the behavior where output_stream would be set if it had never been changed before. It used to default to nil and use ||= but with the need to default it to $stdout it now checks that value manually. Is something in your environment setting config.output_stream before ::RSpec::Core::CommandLine.new(argv).run(stderr, stdout) runs?

Anyhow, I think you're right that it's confusing that there are two ways to set these (one way via run and one way via RSpec.configure) and I would like to simplify to just RSpec.configure for these. If we do make this change, we'll have to add deprecation warnings in 2.99. Also, in #1192 we're discussing some larger changes to RSpec::Core::CommandLine to better support alternate runners. This change could be part of that. Or it could be a standalone change that builds off of.

Do you want to take a stab at a PR removing those arguments?

@untidy-hair
Copy link
Author

Do you want to take a stab at a PR removing those arguments?

Sure. I will see 1192, too.

It's unclear to me why ::RSpec::Core::CommandLine.new(argv).run(stderr, stdout) is failing

When spork command runs, looks like $stdout is STDOUT and @configuration is setup with the value at the time.
But when rspec command runs, $stdout is a different value and @configuration cannot be overwritten with RSpec::configuration. (Because, @configuration ||= RSpec::Core::Configuration.new )
And in the #run method,

@configuration.output_stream = out if @configuration.output_stream == $stdout

if clause returns false because $stdout is not STDOUT any more.
I'm guessing that's why. Not 100% sure, though.

And that's my case so I actually don't know if others see it or not.

Anyway, I think it's better that io err and out arguments are not mandatory for #run , so I will work on the PR.

@untidy-hair
Copy link
Author

Hi there, I couldn't take much time for this, but I found that I have to understand more solidly how and when all configurations are set up in every situation.

I'm going to keep working on this, but if someone has good ideas, please don't hesitate to make a PR.
(I'm sorry but it might take long for me to do it.)

My original idea was basically simple like this:

# in spec/rspec/core/command_line_spec.rb
#    describe "#run with custom output" do

  it "shows a deprecation message" do
    expect(RSpec).to receive(:deprecate)
    command_line = build_command_line
    command_line.run err, output_file
  end

BTW, error_stream should be assigned in RSpec::Core::Configuration.new, too?? Just Wondering.

@myronmarston
Copy link
Member

@untidy-hair -- sorry about the delay, but it's still unclear to me if there's a problem here in rspec or not. A reproducible test case demonstrating the problem would help tremendously. For now I'm going to close this as stale.

@agis
Copy link
Contributor

agis commented Apr 17, 2015

Same issue here, after upgrading to rspec 2.99.0 and using spork 0.9.2. @untidy-hair have you dig any further into it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants