Add options to make all methods as stubs, returning null by default

[yeka]

I'm referring to this documentation of PHPUnit for creating mocked object that all methods are stubs, which return null by default: Unit Testing Tutorial (Do not call setMethods() & Passing an empty array)

This kind of feature doesn't seems currently available on Mockery.

One option that close to that is shouldIgnoreMissing(), but this option will always return null even when the method being called is not exists. This is not good when there's a typo in either unit test's class or actual class.

The additional option of makeStubs() works alike, but will still throw error when a nonexistent method is called.

[coveralls]

Coverage decreased (-0.15%) to 74.25% when pulling 54a9ce0 on yeka:all_stubs into 2448c5b on padraic:master.

[davedevelopment]

So we have the second part you desire as a global flag Mockery::getConfiguration()->allowMockingNonExistentMethods(), if you really only want it on a per mock basis, perhaps we should make that as a separate method available on the individual mocks.

$foo = m::mock("Foo")
    ->shouldIgnoreMissing()
    ->disallowMockingNonExistentMethods();

[ykguru]

Neither work as I expected (couldn't find disallowMockingNonExistentMethods() btw).
Lets say here's my Foo class:

class Foo {
    public function bar() {
        return 'bar';
    }
}

Using Mockery::getConfiguration()->allowMockingNonExistentMethods(false), when calling $foo->bar() it returns null (as expected). But the problem is, I can call $foo->ghost() and it also returns null. I was expecting an error because Foo doesn't have ghost() method.
Or is this a bug?

[ykguru]

Sorry about the disallowMockingNonExistentMethods(), I failed to see that it was a suggestion :p

[coveralls]

Coverage decreased (-0.38%) to 74.02% when pulling 41338f8 on yeka:all_stubs into 2448c5b on padraic:master.

[davedevelopment]

Ah, it seems the shouldIgnoreMissing doesn't respect the allowMockingNonExistentMethods. I'd say it was a bug, but I guess you could argue either way.

[ykguru]

On my latest commit, I got that part covered.
I've also add some unit tests for that.

[GrahamCampbell]

Needs rebasing so we're running with the new test setup.

[yeka]

Rebased

[coveralls]

Coverage decreased (-0.48%) to 73.92% when pulling fafc7c2 on yeka:all_stubs into ef3ca8f on padraic:master.

[GrahamCampbell]

Please fix the cs issues as shown by StyleCI.

[GrahamCampbell]

You can download the diff by clicking the details link, and apply with using git if you want, or you can just manually apply the fixes.

[GrahamCampbell]

Adding methods to an interface is a breaking change btw. We'd need a 0.10 release for this.

[davedevelopment]

We don't need to, there's no BC guarantee until 1.0 IMO

[yeka]

@GrahamCampbell I got your point.
The disallowMockingNonExistentMethods() can be removed from the interface and become a unique method for Mockery\Mock class.

But for Mock class to be able to locally (per mock basis, without setting it from global configuration) disallow mocking non existent methods, the Mockery class needs to have a way to read the local flag. Thus, adding mockingNonExistentMethodsAllowed() to the MockInterface is inevitable.

I'll try to look for a better way implementing this. Hopefully I can find one that doesn't need to change the interface :D

[coveralls]

Coverage decreased (-0.48%) to 73.92% when pulling a17a30e on yeka:all_stubs into ef3ca8f on padraic:master.

[yeka]

I think the disallowMockingNonExistentMethods() for individual mock can wait.
I create another pull request to fix the issue related with global configuration.
shouldIgnoreMissing() and Mockery::getConfiguration()->allowMockingNonExistentMethods() should now work as a team

[padraic]

When I see extra method calls, I start thinking we should simplify. Mockery::stub()? Should we be making this a first class citizen?

[yeka]

I'm not really sure how you're gonna make Mockery::stub() works, is it the same like Mockery::mock()?
On my works, the need for stubs is more common than mocking a non existent methods.
If you could elaborate your thoughts on implementing this Mockery::stub(), I would be more than happy to help write the code :D

[padraic]

@yeka Merely a thought. I'm not sure how possible or not it is without checking.

[ykguru]

Just a thought, how about making mock object as stubs by default?
So when we mock an object, rather than not be able to call its method because we haven't define its behavior, instead it will return null by default (or any defined default return value).
Let me know what you think and concern (if any) regarding this.

[davedevelopment]

Do you mean like Mockery::spy()?

[yeka]

No, Mockery::spy() is a shortcut for shouldIgnoreMissing().
My thought was changing the default behavior of Mockery::mock() so with that line alone, we would be able to call existing function and return null by default.

Given that my Foo class has a method bar()

Current behavior:

$foo = Mockery::mock('Foo');
$foo->bar();

This will result in BadMethodCallException

Suggested default behavior:
$foo->bar() will return null instead of exception.

I don't know if many people make use of current behavior. But I'm just telling the idea anyway.

[davedevelopment]

IMO, that wouldn't be a mock any more it would be a spy, that's why we have Mockery::spy(). It looks to me like your desired behaviour would be solved by using Mockery::getConfiguration()->allowMockingNonExistentMethods(false) in your bootstrap and Mockery::spy().

I'm not keen on only being able to stub/mock existing methods, as it makes interface discovery more difficult for me, instead I prefer to use Mockery::getConfiguration()->allowMockingNonExistentMethods(false) on a CI run, when I've finished with the design phase of TDD.

[yeka]

Understood.
Yeah, my desired behavior should be solved by using Mockery::getConfiguration()->allowMockingNonExistentMethods(false) and Mockery::spy(), except that it doesn't work as expected.

Btw, could you give some thought to my another pull request?

[davedevelopment]

Sorry, I should have said "once your PR gets merged", just 👍'd that PR, will let @padraic have his say but I'm ready to merge.

[yeka]

Thanks, I'm a bit relaxed now :p

[coveralls]

Coverage decreased (-0.62%) to 73.7% when pulling adbd764 on yeka:all_stubs into d141c5b on padraic:master.

[coveralls]

Coverage decreased (-0.62%) to 73.7% when pulling 45ca2e2 on yeka:all_stubs into 5311940 on padraic:master.

[coveralls]

Coverage decreased (-0.62%) to 73.7% when pulling 77510f8 on yeka:all_stubs into a481710 on padraic:master.