2

Let's say you have some class like

class Foo
  ...
  public def methodA
    x = methodB(true)
    # other operations (assume x is not the return value of methodA)
  end

  private def methodB(arg)
    if arg
      return 1
    else
      return 0
    end
  end
end

When you're writing a unit test for methodA, you want to check that x was assigned the right value, which means you have to check that the call to methodB returned 1 like you expected.

How would you test that in rspec?

Right now I have (with help from How to test if method is called in RSpec but do not override the return value)

@foo = Foo.new
expect(@foo).to receive(:methodB).with(true).and_call_original

But I don't know how to verify the actual return value of methodB.

notacorn
  • 3,526
  • 4
  • 30
  • 60
  • 2
    I would argue that you cannot test `methodA` properly in the given example. Because even if you test that `method_b` returns an expected value, you still didn't test that `x` had that value assigned. And furthermore, these are all implementation details. What if `x` gets renamed? What if `methodB` gets refactored? Your tests should not care about the internals of methods or private methods. Instead, test the response of the entire `methodA` or its side-effects. – spickermann May 27 '21 at 18:04
  • `methodA` doesn't return anything meaningful. It's what happens inside it that I want to verify. FWIW idrc that it's "assigned" to `x`, I just want to make sure that throughout the course of `methodA`, when `methodB` is inevitably called, it returns what I expect - also wrt side-effects, `x` is locally scoped, so there is no other way for me to check it, like you might with a class attribute or something – notacorn May 27 '21 at 18:05
  • 1
    @notacorn : The situation you describe basically is about `methodB` returning a certain result for a givin argument. Whether you call `methodB` with this particular argument from inside `methodA` or from a different place, should hopefully make no difference. Hence I would simply write a separate test case involving `methodB` only. – user1934428 May 28 '21 at 06:55

2 Answers2

2

I think instead of mocking you can just call the private method?

@foo = Foo.new
result = foo.send(:methodB, true)
expect(result).to eq 1

This is basically testing the private method directly. Some people frown upon doing that but if it has lots of logic it's sometimes easier to test it directly. I agree with @spickermann that it's usually best to test the public method and leave the implementation details of the private methods out of the specs.

Joel Blum
  • 7,750
  • 10
  • 41
  • 60
  • since this is a unit test for `methodA`, I don't want to directly invoke `methodB`, only `methodA`. I'm really just asking if it's possible to check that return value. – notacorn May 27 '21 at 18:08
  • 1
    Where is the difference between testing `methodB` directly and calling `methodA` but only to check what `methodB` returned to `methodA`? – spickermann May 27 '21 at 18:13
0

Don't Test Multiple Methods in a Single Unit Test

You're going about this wrong, because you're creating an unnecessary dependency between two different methods. Instead, you should refactor your code so that:

  1. x is an argument to #methodA,
  2. x, @x, or @@x is a variable accessible to methodA that you can set to whatever expected value you want, or
  3. stub out #methodB for this unit test.

As an example of the last:

describe Foo do
  describe 'methodA' do
    it 'does something when methodB returns 1' do
      # stub the default subject, which is Foo.new
      allow(subject).to receive(:methodB) { 1 }
      expect(subject.methodA).to eq('foo bar baz')
    end
  end
end

This code is untested, because your posted code is not a complete, verifiable example. While you'd really be better off refactoring your code so you aren't testing nested depedencies at all, using test doubles or method stubs are certainly viable options.

Todd A. Jacobs
  • 81,402
  • 15
  • 141
  • 199
  • so you're saying the class Foo is written incorrectly? `methodB` is a helper method to extrapolate some of the variable interpolation so I don't really see why using a private helper to assign a local variable is "wrong" - I don't see how you refactor that, as the arguments for `methodB` which I excluded from this example, are determined variably in `methodA` as a precursor – notacorn May 27 '21 at 18:20
  • so basically what I'm saying is that (1) and (2) don't really seem like viable solutions, maybe (3) is the only answer. I'm still curious if what I am asking for (originally, anyway) is possible in rspec – notacorn May 27 '21 at 18:20
  • @notacorn What I'm saying is that you didn't post a minimal, complete, verifiable example. Based on what you *did* post, you're testing more than one method at a time. Don't do that. The problem you're having is a common one when people don't write their tests before writing the implementation. – Todd A. Jacobs May 27 '21 at 18:23
  • How is `Foo` not a minimal complete verifiable example? I simply excluded the `initialize` method with the `...`, otherwise it would be a completely valid ruby class. – notacorn May 27 '21 at 18:24
  • 1
    @notacorn "it would be a completely valid ruby class" - it is. It also doesn't make sense to test this implementation, because _nothing happens in it_. That assignment to `x` that you want to test, it doesn't matter if it happens or not, because x is not used. – Sergio Tulentsev May 27 '21 at 19:09
  • The assumption is that x is used later. It makes no sense to make up a use case for it, given that I'm supposed to provide a **minimally** reproducible example. It literally doesn't matter, if you understand the intention/context. If you really wanted, I could replace everything with `puts` statements, but the point stands. – notacorn May 27 '21 at 20:10
  • @notacorn: ah, but this is the whole point, you see. If it was used in a `puts`, you could check that the desired side-effect happened (certain content was sent to stdout). That's what me and the others are trying to tell you: test the behaviour, not the implementation details. – Sergio Tulentsev May 27 '21 at 20:15
  • I think this is a point of frustration that a lot of people have for SO. Questions are asked about how to do X and none of the answers directly answer X but pivot to Y and Z. I am happy to admit that maybe I don't even need to make an assertion on the return value of `methodB`, but that doesn't answer the original question of how you would do it anyway. – notacorn May 27 '21 at 20:19
  • @notacorn: it's answered in the other question, though: test that method directly. I don't think rspec have facilities for `expect(obj).to receive(:msg).with(true).and_also_it_should_return(1)`. This drive-by testing is not done. At least, not in my experience. – Sergio Tulentsev May 27 '21 at 20:22
  • right, I was moreso looking for an answer that would be more upfront about not so much what I should do, but in this case like you're alluding to, whether it even *can* be done – notacorn May 27 '21 at 20:26