rspec / rspec-support

Common code needed by the other RSpec gems. Not intended for direct use.
https://rspec.info
MIT License
95 stars 103 forks source link

Using Refrigerator.freeze_core with rspec crashes #564

Closed fdr closed 1 year ago

fdr commented 1 year ago

Subject of the issue

I'd like to use Refrigerator.freeze_core to freeze core classes, ideally in both tests and at run-time (along with other classes that ought not to be changed past a certain point). RSpec has some unusual patterns in modifying core classes that makes requiring rspec before freezing inadequate to prevent a crash.

Your environment

Steps to reproduce

In spec_helper.rb, add these to the bottom of the file:

require "refrigerator"
Refrigerator.freeze_core

When running even a trivial test, rspec crashes as Object has been frozen by this point, but rspec still wants to make modifications. This is somewhat normal for some libraries, so the usual workaround is to "require" them first, but rspec seems to need something more.

Here's the stack and exception:

<internal:/home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require': can't modify frozen #<Class:Object>: Object (FrozenError)
    from <internal:/home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/ripper/core.rb:12:in `<top (required)>'
    from <internal:/home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
    from <internal:/home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/ripper.rb:2:in `<top (required)>'
    from <internal:/home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
    from <internal:/home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-support-3.12.0/lib/rspec/support/source.rb:54:in `ast'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-support-3.12.0/lib/rspec/support/source.rb:71:in `nodes_by_line_number'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/snippet_extractor.rb:118:in `location_nodes_at_beginning_line'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/snippet_extractor.rb:96:in `expression_node'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/snippet_extractor.rb:88:in `line_range_of_location_nodes_in_expression'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/snippet_extractor.rb:57:in `line_range_of_expression'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/snippet_extractor.rb:42:in `expression_lines'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/snippet_extractor.rb:31:in `extract_expression_lines_at'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/exception_presenter.rb:231:in `read_failed_lines'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/exception_presenter.rb:166:in `failure_slash_error_lines'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/exception_presenter.rb:153:in `block in failure_lines'
    from <internal:kernel>:90:in `tap'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/exception_presenter.rb:152:in `failure_lines'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/exception_presenter.rb:34:in `colorized_message_lines'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/exception_presenter.rb:257:in `formatted_message_and_backtrace'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/exception_presenter.rb:88:in `fully_formatted_lines'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/exception_presenter.rb:80:in `fully_formatted'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/notifications.rb:200:in `fully_formatted'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/notifications.rb:114:in `block in fully_formatted_failed_examples'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/notifications.rb:113:in `each'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/notifications.rb:113:in `each_with_index'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/notifications.rb:113:in `fully_formatted_failed_examples'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/formatters/base_text_formatter.rb:32:in `dump_failures'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/reporter.rb:209:in `block in notify'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/reporter.rb:208:in `each'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/reporter.rb:208:in `notify'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/reporter.rb:178:in `block in finish'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/reporter.rb:194:in `close_after'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/reporter.rb:174:in `finish'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/reporter.rb:76:in `report'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:115:in `run_specs'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:89:in `run'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:71:in `run'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:45:in `invoke'
    from /home/fdr/.asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/rspec-core-3.12.0/exe/rspec:4:in `<top (required)>'
    from /home/fdr/.asdf/installs/ruby/3.2.0/bin/rspec:25:in `load'
    from /home/fdr/.asdf/installs/ruby/3.2.0/bin/rspec:25:in `<main>'

Expected behavior

RSpec should make all its core-classes changes somewhat more eagerly, or, it can document steps whereby I can force those steps to happen before freeze_core.

Actual behavior

It crashes, and it's not obvious how I can force rspec to make whatever standard library modifications it needs to before freezing.

JonRowe commented 1 year ago

We only make monkey patches when using the monkey patching mode, you can turn this off in your configuration:

RSpec.configure { |c| c.disable_monkey_patching! }

This will be a future default but due to a chicken and egg problem still is enabled by default in RSpec 3.

If you still suffer this problem with this turned off please re-open.

fdr commented 1 year ago

Yeah, I do suffer this problem with it turned off. It's a new project; there is no inertia.

fdr commented 1 year ago

@JonRowe also, I don't think I have the permission to re-open an issue if it is closed by a collaborator.

JonRowe commented 1 year ago

You have to add the code snippet to turn it off in a new project

JonRowe commented 1 year ago

Also the stack trace seems to indicate zeitwerk, if this is a rails project see what happens with mini test, Rails is more likely to modify the core obects than us.

JonRowe commented 1 year ago

Have reproduced this myself without zeitwerk ~,but I'm confused as it seems to be coming from modification to one of our meta classes, which shouldn't be frozen as they aren't core~

JonRowe commented 1 year ago

On further investigation this fixes the snippet above:

require "stringio"
require "ripper"
require "refrigerator"
Refrigerator.freeze_core

So it's not RSpec but Ruby at fault for the modifications, we're just requiring files that we'd expect to work, and we do so when we need them to avoid early poisoning of the environment (and with fallbacks in some cases for when they're not available)..

I'm half inclinced to close this as "won't fix" but if you wanted to go through the code base and collect a list of all our requires then add a feature that preemptively loads them that something we could add to work around this (or you could just do it manually in your project)

fdr commented 1 year ago

wontfix is fine, this issue suffices, and I can contribute it to the roda-sequel stack template project. It has a few remarks for other common software (e.g. puma) that need a poke, though generally the poke is eagerly loading the library via require.

fdr commented 1 year ago

also, thank you.