-
-
Notifications
You must be signed in to change notification settings - Fork 760
Running rspec-core's specs with --order default
produces an error
#1073
Conversation
Looks like there's no output source loaded into the deprecation formatter? |
I'm not getting this locally either on master or 2-14... |
Ah, this occurs with |
Ok this happens because of a new deprecation warning being triggered from stub which is changing the load order of the reporter which is causing outputstream to be nil. |
Specs that use the runner need to use the current sandbox output stream (rather than attempting to create a new one) so that inadvertant use of the config hasn't preloaded any formatters (e.g. deprecation warnings will cause stuff to be initialised which would then not be captured in our specs correctly).
@myronmarston had a chance to look this over? |
@@ -17,7 +17,7 @@ def initialize(options, configuration=RSpec::configuration, world=RSpec::world) | |||
# @param [IO] out | |||
def run(err, out) | |||
@configuration.error_stream = err | |||
@configuration.output_stream ||= out | |||
@configuration.output_stream = out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the change? If output_stream
is set to something non-nil, it seems odd (and potentially wrong) to overwrite it....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're using the runner and want to set an output stream, it seemed silly to ignore it, as if you then rely on the new output stream it doesn't work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, see a6ed0c8 though. The code used to be what you changed it to, but according to that commit it changed to ||=
so as to allow configured output_stream
to take precedence over the default of $stdout
. And that makes sense now that I think about it, too: consider that the args passed to this method originate here:
rspec-core/lib/rspec/core/runner.rb
Line 17 in 5f0e7a0
status = run(ARGV, $stderr, $stdout).to_i |
rspec-core/lib/rspec/core/runner.rb
Line 90 in 5f0e7a0
CommandLine.new(options).run(err, out) |
So the out
arg will always be $stdout
(at least from the code path as invoked by the rspec
command), but the user may have configured output_stream
to something else and we don't want to stomp it.
It looks like there's a missing test case for this, though; can you revert this and add one?
Looks good in general. I do have the one question above. Otherwise, merge away. |
Running rspec-core's specs with `--order default` produces an error
When you run:
It fails: