lets block for defining multiple let blocks at once

[JoshCheek]

The code is here.

---

I had a method that returned three values (stdout, stderr, exit status), and setting these values with let blocks was quite obnoxious:

let(:results) { call "args" }
let(:stdout)  { results[0] }
let(:stderr)  { results[1] }
let(:status ) { results[2] }

So I wound up using local variables, because then I could just do:

stdout, stderr, status = call "args"

This had drawbacks, though:

• If it blew up, it did it at definition time rather than test time, so feedback about where/why was poor (rather than this test failing, it just exited Ruby)
• It kept sharing variables across contexts, so by the time the test got to running, the value of the variable had changed (yeesh, that's annoying!)

So I decided I'd like to be able to have a lets block, which is the same as a let block, except it sets multiple names at once so that it works more like the local variable example. Now I can do:

lets(:stdout, :stderr, :status) { call "args" }

Figured I'd post it here, and see if it fit with what y'all are trying to do, before trying for a PR.

[cupakromer]

I've thought about this a bit. Unfortunately, this isn't something that I'd want to pull into core; I'll let the rest of the team chime in on their thoughts.

I have several reasons for this, the most concerning is what we would need to do for all the edge cases. What is the behavior when the defined helper names don't match directly to the splatted return value? As in your gist, some people may want an early failure. Others may want the assignments to be nil per normal Ruby behavior. Additionally, if there are too few helper names do the remaining values get bundled into an Array per normal Ruby or are they dropped on the floor?

What if I want to slurp up the values in another position? Does that mean we now need to support parsing each name to check for a splat?

# Not to mention this requires you to use a string b/c Ruby
# doesn't like :*ios due to parsing rules
lets(:'*ios', :status) { call "args" }

# What if I don't care about the values? Do we support this?
# Or some alternative?
lets(:stdout, :*) { call "args" }

This also brings up the complexity about what if I don't want one of the splatted values? Is the syntax then:

lets(:stdout, :_, :status) { call "args" }

What if I do the following?

lets(:stdout, :_stderr, :status) { call "args" }

Depending on coding style that could either mean "I don't intend to use stderr but I'm describing what I'm skipping", or it could mean "I mean for stderr to be used privately only by this class". In most cases the context (i.e. method name vs local variable name) determine what the convention means; here it's ambiguous. On a second read the latter meaning doesn't really make sense in the context of specs.

Splats are not the simplest to understand in Ruby (I often have to recheck syntax and try it in pry when I use them). Add to that, let tends to be confusing enough for many people as it is. I think combining these two features is a setup for more headaches for users.

[cupakromer]

As for an alternative, I think it really depends on usage. I'm having a hard time thinking of an instance when I actually wanted to do this. I would not suggest "context" local variables, which it seems you tried to use. I know others on the core team use them, but I only use them to define things like scoped classes, modules, to "constants" (which I aggressively call freeze on).

As you found out, "context" locals are not cleaned up (one of their major downsides) and can mutate between specs. This is simply due to Ruby block-scoping rules. Below is a code sample that shows this:

describe "defining the context" do

  this_is_a_context_local = []

  it "can mutate state" do
    this_is_a_context_local << :modified
  end

  it "will fail or pass depending on run order" do
    expect(this_is_a_context_local).to eq []
  end

  it "this may be confusing for some" do
    # Due to block scoping rules I just changed
    # the actual variable and now it means 123
    # for subsequent specs that run
    this_is_a_context_local = 123
  end

  it "if this runs after the above spec it passes" do
    expect(this_is_a_context_local).to eq 123
  end

end

I always tell newer Ruby-ist to start with spec local variables. Often they try to abstract out into let too soon and get themselves into trouble. By starting with "spec" locals, you can freely move and adjust specs until a clear structure emerges. When you want to extract out into a let you can also ask yourself:

Does this let really mean the same thing for each of the following specs? Or am I just trying to reduce coincidental duplication? If I set this value differently in spec A and in spec B does that make the specs meaning change?

If not, maybe a let isn't the proper tool.

For your specific use case, it's hard to say without more concrete example specs. Honestly, I probably would just use "spec" local variables. Though, since it seems this was very much related to running a process, I would probably move the "action" into either a shared helper method, or just re-call it in each spec. I can then use the new RSpec output matchers. Or I can use something like Process.spawn or Open3 to have more fine grain controller things.

Another alternative, since RSpec just creates classes in the background and all let does is define a method, is to use a helper method with some attr_readers:

describe "a context" do
  attr_reader :stdout, :stderr, :status

  def run_the_process(args)
    @stdout, @stderr, @status = call(args)
  end

  it "exits successfully" do
    run_the_process("args")
    expect(status).to be_success
  end

end

[myronmarston]

I saw your tweet to your gist yesterday, @JoshCheek, and seeing lets(:first, :second) { [1, 2] } in the examples, I didn't really see the point of it. Seeing an example like lets(:stdout, :stderr, :status) { call } makes this look a lot more reasonable to me. I can definitely see it's usefulness for a method that returns multiple values (such as shelling out).

That said...I can't think of a single time I've ever wanted something like this (then again, maybe I've had situations that could have benefitted from this but didn't realize it at the time). Given that this is intended to be used in a similar fashion to destructuring assignment, I think I'd expect it to have the same semantics...but as @cupakromer has pointed out, there are definitely some complexities with this, particularly with how splats work.

I also think that lets is not a very good name; it reads to me like "let's" or "let us", which doesn't really make sense.

Historically, I think RSpec has been too eager to add new features/APIs suggested by users. Over time, we've learned that some of those features are a better fit for external gems (like its) and at this point, I think our users are better served by a higher standard of how commonly needed/useful a new feature is before we incorporate it in RSpec. It serves our users better for new feature suggestions like this to begin life as an external gem and later incorporate it (like we did with rspec-fire), rather than pulling a feature in prematurely and then later extracting it (as we did with its).

So...I'd encourage you to package this in an extension gem and if the community finds it widely useful and the corner cases get worked out, we can consider pulling it in the future...but for now, I don't think this belongs.

[JoshCheek]

• These are all good points
• I think the before { @stdout, @stderr, @status = call "args" } makes a lot of sense.
• The "too-many/too-few/splatting/underscoring/destructuring" is a solved problem. Friends, I'm proud to present you with the RSpec lets expansion pack Your children's children will sit hushed and entranced as you tell them stories of this day over a crackling summer fireplace -- lightning bugs trailing fire through the sky behind you, if to tell them "yes, little ones, magic is real".
• It's good that you're increasing the bar for entry into the codebase, as a user (and esp as one who reads a lot of the code in the code in the libs I use), I appreciate this a lot.
• I gotta say, I'll probably never do locals again. I was even trying to be careful! But just wound up really confused, there's weird behaviour with these ones. Here's a minified example:

class Program
  # ...
end

if $0 !~ /rspec/
  # the out here is visible below, so gets shadowed, and they share state
  out, err, status = Program.call ARGV
  $stdout.print out
  # ...
else
  RSpec.describe Program do
    # ...
    context 'one' do
      out, err, status = Program.call [...] # => ["out1", ...]
      # this will fail, because the context below updated what out points to
      it('is out1') { expect(out).to eq 'out1' }
    # ...
    context 'two' do
      out, err, status = Program.call [...] # => ["out2", ...]
# ...

[skatenerd]

I personally ran into this issue when trying to refactor away from let statements and towards explicitly invoking helper methods to set up my data. Specifically, I was addressing this pattern:

describe 'foo' do
  let(:age) {100}
  let(:user) { User.create!(age: age) }
  let(:analyzer) { Analyzer.new(user) }
  describe 'bar' do
    let(:age) {300}
    it "is very old" do
        expect(analyzer.get_user_age()).to eql(300)
        expect(analyzer.get_user_age()).to eql(user.age)
    end
  end
end

I thought this was really tricky/fragile, as it relied on laziness, and because I couldn't understand the test itself without looking at all of the "parent" contexts and simulating in my brain how all of the different setup code would interact. So, I thought to myself, I know how to make things explicit: using arguments. I created a helper method which accepts an 'age' argument and returns the analyzer and the user. That's when I started googling and found this issue.

The approach I took was to just write code like:

it "foo" do
  analyzer, user = setup(300)
  ...
end

The main frustration I have is that, if I have more setup code, and the setup code requires access to analyzer and user, then that setup code can't use let! Once I've gone down the path of splat-into-local-variables-by-hand, suddenly let is totally off limits and I get a lot of repetition in my tests.

As for the semantics of how you would implement this, I feel like you would want call "args" to return a hash. After that point, you can't go wrong with crashing often and early. If the user-provided variable names (on the left-hand-side) don't align with the keys of the returned hash, then the test is invalid.

[JonRowe]

👋 As per the original comments at the time, this is still something we'd be reticent to bring into core, there is nothing stopping you from implementing this as an extension in the mean time. let just defines methods with memoization of the values from the block. Roughly heres how you could implement this yourself:

module MultiLet
  def let(*names, &block)
    if names.size == 1
      super(names[0], &block)
    else
      metaname = :"__#{names.join("_")}"
      let(metaname, &block)
      names.each do |name|
        super(name) do
          begin
            send(metaname).fetch(name)
          rescue KeyError
            raise ArgumentError, "#{name} not in block results"
          end
        end
      end
    end
  end
end

RSpec.configuration.include MultiLet

Thats overriding let and using the original let interface as its a public api, you could do away with the meta let if you didn't need to delay the block call etc

[skatenerd]

@JonRowe Thanks for the tip!!