rspec / rspec-core

RSpec runner and formatters
http://rspec.info
MIT License
1.23k stars 765 forks source link

Define weak/default memoized helpers with `let?` method. #3077

Closed sleepingkingstudios closed 7 months ago

sleepingkingstudios commented 7 months ago

Add Weak Memoized Helper

When writing reusable tests, I frequently want to be able to set default values for memoized helpers. This ensures that there is a helper defined, but does not overwrite a previous value if there is one. Here is a (toy) example of how this might be done currently:

RSpec.describe Processor do
  shared_examples 'processes the input' do
    let(:options) { {} } unless method_defined?(:options)

    it 'processes the input' do
      expect { subject.process(input, **options) }.not_to raise_error
    end
  end

  let(:input) { 'Greetings, programs!' }

  include_examples 'processes the input'

  context 'with options' do
    let(:options) { { capitalize: true } }

    include_examples 'processes the input'
  end
end

This is kind of clunky, and I don't think defining memoized helpers conditionally is actually idiomatic RSpec.

Current Options

Use Conditional Memoized Helpers

As above, kind of clunky and probably not idiomatic (even if it's valid Ruby). I'd probably have words with a junior dev who put this in a Pull Request.

Use super():
let(:options) { defined?(super()) ? super() : {} }

More idiomatic, but I don't entirely trust calling super() twice - it could potentially cause issues depending on how memoization is handled and what super() actually does. Worse, this fails confusingly when there are multiple shared example groups included in an example group. (I have a proposal for an alternative for sharing tests, but it's a long writeup by itself).

Proposal

Define a let? memoized helper method, which functions as let but only defines the helper if there is not already a method defined with that name.

shared_examples 'processes the input' do
  let?(:options) { {} }

  it 'processes the input' do
    expect { subject.process(input, **options) }.not_to raise_error
  end
end

This could potentially interact with non-helper methods (defined using def or define_method), but I think that's actually correct for this use case - if someone defines a def options; end method, we probably don't want to overwrite it with our default value anyway.

I'm not married to the exact syntax, if there's a preferred nomenclature to use for this use case. I'm happy to implement this if there is interest (if there isn't, I'll add it to my helper gem and call it a day). Please let me know if there's a more appropriate forum for discussing features like this before they hit a GitHub issue as well. Thanks (and thanks to everyone for your hard work on this project - RSpec is absolutely essential).

pirj commented 7 months ago

After a semi-relevant recent discussion in https://github.com/rspec/rspec-core/issues/3076, I have doubts if we would want to add such a feature in the core.

I can only guess why if there’s just one variable, it is not explicitly defined in the calling code, and is set a default value in the shared example group. Would it be used to ease up the test setup for describing a subject dependent on many variables? Without more specifics, I’d say it would only bury the complexity deeper. I would be most concerned as a reader of a spec to what values of variables are used in each context.

All kinds of extensions exist, built for various purposes, some very useful for performance like let_it_be. I would suggest writing an extension and providing sone convincing examples of making code easier to read.

Personally, I would change your example to

RSpec.describe Processor do
  shared_examples 'processes the input' do
    it 'processes the input' do
      expect { subject.process(input, **options) }.not_to raise_error
    end
  end

  let(:input) { 'Greetings, programs!' }
  let(:options) { {} } # default

  include_examples 'processes the input'

  context 'with **non-default** options' do
    let(:options) { { capitalize: true } }

    include_examples 'processes the input'
  end
end
sleepingkingstudios commented 7 months ago

Fair enough, and I apologize for not checking the closed issues for relevance. If it helps understand the context - my end goal is to be able to define portable tests. For example, a transportation gem might define a shared contract for being a valid form of transport, while the race_car, dirigible, and rocket gems that depend on transportation could then pull in the tests and verify that their implementations match. This isn't a hypothetical for me, it's a problem I'm specifically trying to solve for Collection implementations for cuprum-collections. Still, if there's not much interest in tackling this in core at this time I'll just build it into my own testing helper gem.

pirj commented 7 months ago

No worries. I think I understand your use case, and I have some experience with such specs as a user.

I wonder if there could be a single entry to such shared specs, and if it can be optionally overridden by the code using them?

RSpec.shared_examples “processes the input” do
  let(:foo) { :default }
  it { … }
end

RSpec.describe do
  it_behaves_like “processes the input” # with defaults
  it_behaves_like “processes the input” do
    let(:foo) { :override }
  end
end

Would this be of some help? Does it even work as I remember it should?

sleepingkingstudios commented 7 months ago

I think it works that way. It feels inside out relative to how RSpec usually works, though. I think a lot of the trouble I'm having is working around the use cases where shared example groups are kind of like including a module, except when they're not.

What I really want in the end is to be able to share tests between projects/libraries, but I kind of suspect that's going to be considered way out of scope no matter how elegant I think the solution is. I think the intended way to do that in RSpec is creating a shared example group in the top-level scope which adds it to RSpec.world, but feel free to correct me if I'm missing something obvious.

JonRowe commented 7 months ago

Shared examples are a context [e.g they are a class inheriting from the parent] in their own right where as shared contexts are not, this is why there is a slight difference in how inclusion works. You should be able to do what @pirj suggests and have it just work.

e.g. this won't work because the parent class's definition is overridden.

RSpec.shared_examples “processes the input” do
  let(:foo) { :default }
  it { … }
end

RSpec.describe do
  let(:foo) { :override }
  it_behaves_like “processes the input” # with defaults
end

e.g but this should becase its defined in the class

RSpec.shared_examples “processes the input” do
  let(:foo) { :default }
  it { … }
end

RSpec.describe do
  it_behaves_like “processes the input” # with defaults
  it_behaves_like “processes the input” do
    let(:foo) { :override }
  end
end

You can also check in the shared example for the presence of the method before defining the let

sleepingkingstudios commented 7 months ago

@pirj @JonRowe I think I'm going in a different direction here, but I just wanted to say thank you for taking the time to go through this "the RSpec way". RSpec is always the first thing I add to any new project or recommend, not just because the code is rock solid but because the support and the community are always there, and it's deeply appreciated. Cheers!