add stub_any_instance method #245

Closed
wants to merge 1 commit into
from

4 participants

@nashby

Because sometimes we want to stub some method on all instances of the class.
@zenspider wdyt? Should this be in minitest or it can live as a helper or something? (we have it as helper already in SimpleForm)

@zenspider
Seattle Ruby Brigade member

I think this is better suited as a plugin gem. I'd be happy to help you get that started if you'd like.

After, please send a pull request listing the new gem in the readme and I'll merge and republish my doco so people can find it easier.

@zenspider zenspider was assigned Mar 5, 2013
@nashby

@zenspider well, I decided that this can live as a method in SimpleForm specs. Not sure this one little feature deserve a gem :)

@nashby nashby closed this Mar 10, 2013
@nashby nashby deleted the nashby:stub-any-instance branch Apr 6, 2015
@mig-hub

Just out of curiosity @zenspider , what is the reason for not having this in the core?
It sounds to me that "stub" and "stub_any_instance" are closely related.

I understand that we can't put everything in core but I would find it more useful to have this method instead of some shortcuts like must_be_nil which are not much shorter than must_be :nil?.

This is not a rant by the way, I am pretty sure there is a reason, like a downside or something and I'd like to know what it is.

Cheers,
mig

@zenspider
Seattle Ruby Brigade member

@mig-hub and you can have that method... as a plugin.

@mig-hub

@zenspider, yes I know there is a plugin, I use it sometimes.
I still wanted to know the reason to move it to a plugin instead of having it with stub.

Believe it or not, I am hoping to learn something from you.

I'd like to know if you would use this plugin for your own test or not for example.
Not that it would mean the method is not good, but maybe there is just a simple way to do it with a few lines. I have the feeling this is possible by stubbing :new on the class then stubbing the instance in a nested way. Although I never tried that.

Or maybe you think it is not necessarily a good way to test in the first place, and that would interest me as well.

The thing is that minitest is very simple to understand indeed and at the end of the day, knowing what to test and how to test it is a much harder task, at least for me. I realised that people (including me) often end up wanting more features because they don't necessarily use their tool in the best way possible.

This is the reason why I try to read the tests of minitest itself a lot.

Could you please give me your insight on this subject?

Cheers,
mig

@zenspider
Seattle Ruby Brigade member

It's not a move, because it was never added.

Personally, I never use mocks and I almost never use stubs (and when I do, it's most often via def). stub_any_instance seems like sloppy testing to me and I've never had a need for such a thing so it hasn't proven its value enough to be a core thing to use. Adding stub_any_instance doesn't fit my vision for minitest as a result. I'm starting to cede decisions about minitest/mock to @zamith as he has shown a vested interest in it and he gets minitest.

@zamith

I share @zenspider's opinion that stub_any_instance usually is a symptom of a larger problem, and you should go and fix that. At least that's my experience.

Even if you do use mocks, you should be able to inject a mock and control what's happening. stub_any_instance usually comes with tightly coupled systems. Hope it helps.

@mig-hub

Thank you so much @zenspider and @zamith, this exactly the kind of answer I was after. Mainly because I started to have doubts about it.

There are some edge cases where stub and stub_any_instance break unexpectedly when it is mixed and nested with other concepts. For example I ran recently into a case where stub_any_instance breaks because I tried a mock object instead of the Proc. It works perfectly with stub and allows to check if a method was called, but not stub_any_instance because of this:

define_method(name) do |*args|
  if val_or_callable.respond_to? :call then
    instance_exec(*args, &val_or_callable)
  else
     val_or_callable
  end
end

It breaks because the mocked method responds to :call, that's its job, but can't used with &. There might be a work around, but this kind of thing makes me think that it is too twisted and that there must be a simpler way of making sure the code works.

I find the balance between testing only one thing at a time and not testing the implementation very hard. I probably would trade any testing framework for a good book which contains the truth about testing. Only half joking here, but this is why I value the code maintainers' vision as much as the code itself.

Anyway, sorry for keeping this outdated thread alive. Thank you guys for taking the time to answer my questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment