Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Command line --require are not visible in spec_helper.rb configuration block #3116

Closed
trinistr opened this issue Oct 7, 2024 · 2 comments · Fixed by #3117
Closed

Command line --require are not visible in spec_helper.rb configuration block #3116

trinistr opened this issue Oct 7, 2024 · 2 comments · Fixed by #3117

Comments

@trinistr
Copy link

trinistr commented Oct 7, 2024

Files required on command line are not visible or available in spec_helper.rb configuration block

I'm working on a project where BigDecimal support is optional and ideally tests would work both with require "bigdecimal" and without it. To that end, it would be useful to skip tests with BigDecimal depending on whether "bigdecimal" was required. However, there seems to be no clean way to do it using metadata and --require, which I thought would be the correct way to go about this.

There are actually two different issues I've noticed:

  1. With default .rspec files required on command line are required after configuration block has already been executed.
  2. RSpec.configuration.requires seems to always be empty in configure block.

Your environment

  • Ruby version: ruby 3.1.4p223 (2023-03-30 revision 957bb7cb81) [x86_64-linux], ruby 3.3.5 (2024-09-03 revision ef084cc8f4) [x86_64-linux]
  • rspec-core version: rspec-core 3.13.1

Steps to reproduce

Variation 1
These steps reproduce both issues 1 and 2.

.rspec

--require spec_helper

spec_helper.rb

RSpec.configure do |config|
  config.disable_monkey_patching!

  config.expect_with :rspec do |c|
    c.syntax = :expect
  end

  puts RSpec.configuration.requires.inspect
  # Skip tests using BigDecimal if no support is available.
  unless defined?(BigDecimal)
    config.filter_run_excluding(bigdecimal: true)
    warn "no BigDecimal support detected, BigDecimal tests will not be run"
  end
end

spec/kernel_spec.rb

RSpec.describe Kernel do
  describe "#BigDecimal", :bigdecimal do
    puts RSpec.configuration.requires.inspect

    let(:conversion) { BigDecimal(number) }

    context "with an object" do
      let(:number) { Object.new }

      it "raises an error" do
        expect { conversion }.to raise_error TypeError
      end
    end
  end

  describe "#puts" do
    it "outputs a string" do
      expect { puts "string" }.to output("string\n").to_stdout
    end
  end
end

Running rspec shows that bigdecimal was not required when needed, though it is present in requires in examples:

$ rspec --require bigdecimal
[]
no BigDecimal support detected, BigDecimal tests will not be run
["spec_helper", "bigdecimal"]
Run options: exclude {:bigdecimal=>true}
.

Finished in 0.0011 seconds (files took 0.08006 seconds to load)
1 example, 0 failures

Variation 2
This variation reproduces issue 2 (empty requires).

.rspec

--require bigdecimal
--require spec_helper

Running rspec shows that "bigdecimal" was required and executes previously excluded test, but requires is still empty in configure and filled in specs themselves:

$ rspec
[]
["bigdecimal", "spec_helper"]
..

Finished in 0.00314 seconds (files took 0.07711 seconds to load)
2 examples, 0 failures

Expected behavior

  1. RSpec.configure should probably be ran after processing all requires.
  2. config.requires in configure block should not be empty.

Actual behavior

  1. configure is executed immediately on loading spec_helper.
  2. config.requires is filled after configure was already executed.

While requiring a library in .rspec works for actually running tests, it is very inconvenient, as the same result could be achieved by changing spec_helper.rb, which is exactly the problem I was trying to avoid.

@JonRowe
Copy link
Member

JonRowe commented Oct 7, 2024

👋 So there is a bit going on here...

Because you are using .rspec to load your spec_helper.rb it will always run before the CLI requires, this is a design decision to allow CLI options to override file based options which is desirable for overridable settings, but obviously requires are just added together, so there is both a load order issue and a feature/bug in how we record required files, which is we don't set them in the config until after we've required them. I actually worked up a quick patch to that issue to see if would help you but ran into the load order at that point...

Additionally RSpec.configure is just a helper for essentially RSpec.configuration.tap do and has no magic, because of some chicken and egg issues with config it would be quite difficult to delay execution of these blocks until later and then that might cause confusion and surprise given the way it has behaved for decades.

That said, I have a suggestion on how to solve this for your use case which is to switch to using a skip directive in your spec_helper.rb

RSpec.configure do |config|
  config.disable_monkey_patching!

  config.expect_with :rspec do |c|
    c.syntax = :expect
  end

  config.before(:context, bigdecimal: true) do
    unless defined?(BigDecimal)
      skip "no BigDecimal support detected, BigDecimal tests will not be run"
    end
  end
end

@trinistr
Copy link
Author

trinistr commented Oct 8, 2024

Thank you! Didn't realise that skip isn't only a metadata directive, now it's obvious that it should work in a similar way.

Additionally RSpec.configure is just a helper for essentially RSpec.configuration.tap do and has no magic

Completely understandable, having magic in code is a path to madness.

I see a PR was opened to address issue 2, so I'll leave this issue open.

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

Successfully merging a pull request may close this issue.

2 participants