Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support symbols as stub names #14

Closed
pyrsmk opened this issue Oct 1, 2020 · 9 comments
Closed

Support symbols as stub names #14

pyrsmk opened this issue Oct 1, 2020 · 9 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@pyrsmk
Copy link

pyrsmk commented Oct 1, 2020

It would be great to be able to use symbols as stub names for mocks.
But I know this a complicated edge case ^^

My specific need comes from my Origin shard (https://github.com/pyrsmk/origin) which allows the delegation of methods with complex names like []? or <<. When implementing it in my code, I want to test that the delegated methods are well-delegated ^^

Here's a concrete and simplified example:

# A node with delegated methods from XML::Node.
struct Node
  autowire text : String, :[], :[]?

  def initialize(@origin : XML::Node); end
end
Spectator.describe Node do
  let(text_value) { Random::Secure.hex }
  let(array_get_value) { Random.rand(Int32) }
  let(array_get_or_nil_value) { [Random.rand(Int32), nil].sample }

  mock XML::Node do
    stub text
    # This will raise a syntax error at compile-time.
    # stub :[]
    # stub :[]?
  end

  let(html) { "<!doctype html><html></html>" }
  let(xml_node) { XML.parse_html(html) }
  subject { described_class.new(xml_node) }

  before_each do
    allow(xml_node).to receive(text).and_return(text_value)
    # allow(xml_node).to receive(:[]).and_return(array_get_value)
    # allow(xml_node).to receive(:[]?).and_return(array_get_or_nil_value)
  end

  it "wires #text method" do
    expect(subject.text).to eq text_value
  end

  # it "wires #[] method" do
  #   expect(subject[0]).to eq array_get_value
  # end

  # it "wires #[]? method" do
  #   expect(subject[0]?).to eq array_get_value_or_nil
  # end
end

There is a strong limitation (coming from Crystal): we cannot define those methods with a return type. So the only case of such implementation would be with mocks but not doubles (or only partially).

@icy-arctic-fox icy-arctic-fox self-assigned this Oct 1, 2020
@icy-arctic-fox icy-arctic-fox added the bug Something isn't working label Oct 1, 2020
@icy-arctic-fox
Copy link
Owner

This is kind of a new feature, but I would expect it to work with symbols anyways. What's problematic here is that there's no syntactically correct way to represent a stub for an operator. What I have done is add support for this syntax:

struct Node
  def text
    "foo"
  end

  def [](index)
    42
  end

  def []?(index)
    42.as(Int32?)
  end
end

Spectator.describe Node do
  mock Node do
    stub :text { "bar" }
    stub :[], index { 123 }
    stub :[]?, index { 123.as(Int32?) }
  end

  let(node) { Node.new }

  it "stubs the mock" do
    expect(node.text).to eq("bar")
    expect(node).to have_received(:text)
    expect(node[0]).to eq(123)
    expect(node).to have_received(:[])
    expect(node[0]?).to eq(123)
    expect(node[0]?).to be_a(Int32?)
    expect(node).to have_received(:[]?)
  end
end

If the method arguments have type restrictions, then the following could be used:

stub :[], index : Int32 { 42 }

This should be available shortly.

icy-arctic-fox added a commit that referenced this issue Oct 2, 2020
Works like:
  stub :[], index : Int32 { 42 }

Addresses #14
@pyrsmk
Copy link
Author

pyrsmk commented Oct 2, 2020

Wow, that was quick, thank you!

I had a second thought about the return type, instead of an impossible type declaration we could use a simple optional argument like:

stub :[], index, return_type: Int32 { 123 }

Another thing, when writing those tests, I struggled with passing a let variable to the body of the stub:

let(text_value) { Random::Secure.hex }

mock XML::Node do
  stub text { text_value }
end

This was raising an error because it couldn't find text_value. So I ended up adding allow(xml_node).to receive(text).and_return(text_value). Is this the right way to handle it? Is there something that can be done to handle it and avoid writing that extra line? I can create another issue if needed.

@icy-arctic-fox
Copy link
Owner

That's a good way to specify a return type.

And yes, that is a known limitation (probably not documented though). The block for mock stubs runs in the context of the mocked type. It can't see anything in the spec, including let and subject. Defining it with and_return is one way of doing it. Another is with a top-level "global," which I don't recommend.

@icy-arctic-fox
Copy link
Owner

What is your opinion on the stub block? It could be executed in scope of the spec or the mocked type, but not both. Which would be more beneficial for you?

@pyrsmk
Copy link
Author

pyrsmk commented Oct 3, 2020

I think that when we're defining a block we're implying that this block is run within the current context (aka the spec). Because this is what happens usually.

Do you have a real world example of a stub that needs to run in the context of the mocked type?

@pyrsmk
Copy link
Author

pyrsmk commented Oct 3, 2020

That's a good way to specify a return type.

That's how I implemented it in Origin recently. Since it's an edge case, the syntax is not too obtrusive IMO.

@icy-arctic-fox
Copy link
Owner

I think that when we're defining a block we're implying that this block is run within the current context (aka the spec). Because this is what happens usually.

Do you have a real world example of a stub that needs to run in the context of the mocked type?

Thinking more on it, having the spec scope would be better. There might be some technical problems to work out.

@icy-arctic-fox icy-arctic-fox added the enhancement New feature or request label Nov 7, 2020
@icy-arctic-fox
Copy link
Owner

The return_type option has been implemented for stubs. This has been released in v0.9.28. The following now works:

struct Node
  def text : String
    "foo"
  end

  def [](index) : Int32
    42
  end

  def []?(index) : Int32?
    42.as(Int32?)
  end
end

Spectator.describe Node do
  mock Node do
    stub :text, return_type: String { "bar" }
    stub :[], index, return_type: Int32 { 123 }
    stub :[]?, index, return_type: Int32? { 123.as(Int32?) }
  end

  let(node) { Node.new }

  it "stubs the mock" do
    expect(node.text).to eq("bar")
    expect(node).to have_received(:text)
    expect(node[0]).to eq(123)
    expect(node).to have_received(:[])
    expect(node[0]?).to eq(123)
    expect(node[0]?).to be_a(Int32?)
    expect(node).to have_received(:[]?)
  end
end

I will create a new issue for changing the scope of stubbed methods.

@icy-arctic-fox
Copy link
Owner

House keeping 🛎️ Please reopen if this issue hasn't been addressed or suits your needs.

Stubbed methods can be a symbol (:[] and :method). Additionally, a return type can be specified manually with the return_type: TYPE option.

This syntax has been quite clumsy. I'm planning to revisit the mock/stub DSL in an upcoming version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants