Ensure Bisect::Channel is able to send data successfully when UTF-8 encoding is set

[mikejarema]

This PR is the outcome of debugging an issue that arose when running rspec --bisect on a Rails project.

My environment:
Ubuntu 20.04 running under Windows WSL
RSpec 3.10.1
Rails 5.2.4.4
Ruby 2.6.6

One of our specs fails sporadically so I tried using rspec --bisect to find the source of the failure.

$ bundle exec rspec --seed 1638 --bisect
Bisect started using options: "--seed 1638"
Running suite to find failures...

Rspec would hang like this indefinitely. By inspecting ps aux | grep rspec I confirmed that the forked spec runner did complete, but did not report back its results to the parent process.

The problem was in RSpec::Core::Bisect::Channel.

Namely, RSpec::Core::Bisect::Channel#send was silently failing in the forked process with the following error on @write_io.write.

Encoding::UndefinedConversionError: "\xF8" from ASCII-8BIT to UTF-8

The results from the spec run (latest_run_results) would contain a non-UTF-8 character after being sent through Marshal.dump.

Since I'm running these specs in the context of Rails, and Rails by default sets UTF-8 encoding, the IO::pipe itself was set to use UTF-8 encoding. Since the output of Marshal.dump contained binary characters, the @write_io pipe couldn't accept the data.

My PR addresses this by proactively setting the encoding on the @write_io pipe to use the same encoding as the data dumped by Marshal.dump. I've added specs which reproduce, in isolation, the preconditions which caused the failure I saw and ensure that 1) the data is sent to @write_io pipe successfully and 2) that the parent process receives it successfully.

[mikejarema]

After applying this patch locally to rspec-core, I was able to bisect and determine the cause of the spec failure.

[benoittgt]

Very interesting issue. Thanks for the report.

I tested locally your patch and it works as expected.

[JonRowe]

Can you rebase this please

[JonRowe]

Ah so, of course, encoding is not a method for 1.8.7, our usual work around would be to wrap it in a check, I'm about 80% certain 1.8.7s default encoding as just :ascii and in fact IO might not even have set_encoding which would mean you'd need to do a similar check there.

Suggested change

	MARSHAL_DUMP_ENCODING = Marshal.dump("").encoding
	if String.method_defined?(:encoding)
	MARSHAL_DUMP_ENCODING = Marshal.dump("").encoding
	else
	MARSHAL_DUMP_ENCODING = :ascii
	end

[mikejarema]

Makes sense, thanks for the suggestion. I've confirmed that IO#set_encoding doesn't exist in 1.8.7, nor does the global Encoding object.

I've adjusted the code and specs to skip unsupported method calls, relying on whatever encoding is implicit to 1.8.7.

Hopefully this gets the PR across the line. I had trouble building MRI 1.8.7 on my end for testing so I'm relying on CI to reveal any issues. I'll address them as necessary.

[mikejarema]

The legacy ruby issues seem to have been addressed, however a few of the CI environments are failing for odd reasons (timing-sensitivity or "Resource temporarily unavailable").

I can't determine whether they're related to this PR.

[mikejarema]

The last CI failure (jruby-9.1.17.0) seems unrelated to this PR. Any guidance?

An error occurred while loading spec_helper.
Failure/Error: module ArubaLoader

NoMethodError:
  private method `require' called for Kernel:Module
  Did you mean?  java_require
# uri:classloader:/jruby/kernel/kernel.rb:13:in `require_relative'
# ./bundle/jruby/2.3.0/gems/aruba-0.14.14/lib/aruba/platforms/unix_platform.rb:113:in `block in require_matching_files'
# ./bundle/jruby/2.3.0/gems/aruba-0.14.14/lib/aruba/platforms/unix_platform.rb:113:in `require_matching_files'
# ./bundle/jruby/2.3.0/gems/aruba-0.14.14/lib/aruba/api.rb:20:in `<main>'
# ./spec/support/aruba_support.rb:1:in `block in (root)'
# ./spec/support/aruba_support.rb:4:in `ArubaLoader'
# ./spec/support/aruba_support.rb:3:in `<main>'
# ./spec/support/aruba_support.rb:1:in `block in (root)'
# ./spec/spec_helper.rb:1:in `<main>'
# ./spec/spec_helper.rb:24:in `(root)'
# ./spec/spec_helper.rb:17:in `block in (root)'
No examples found.

[JonRowe]

My hot tip is to rebase when #2858 is merged 😂

[JonRowe]

Fix should be merged so if you rebase it should pass.

[mikejarema]

All green, great! Thank you for the fix @JonRowe, and helping with my first RSpec contribution. @benoittgt thank you for reviewing.

[JonRowe]

Thanks!