Add option to emit a warning when non-existant classes/interfaces/methods are stubbed/mocked #12

Closed
sebastianbergmann opened this Issue May 25, 2010 · 11 comments

Comments

Projects
None yet
7 participants
@sebastianbergmann

No description provided.

@igorw

This comment has been minimized.

Show comment
Hide comment

igorw commented Nov 11, 2011

+1

@edorian edorian referenced this issue in sebastianbergmann/phpunit Oct 4, 2012

Closed

getMock should always use munged name by default #678

@edorian edorian referenced this issue in sebastianbergmann/phpunit Nov 24, 2012

Closed

No warning when class to be mocked doesn't exist #728

@edorian

This comment has been minimized.

Show comment
Hide comment
@edorian

edorian Nov 24, 2012

Collaborator

I'd like to do something about this for 3.8.

One option would be to have a new method that only mocks existing things and one to create new stuff and do proper error reporting there "deprecating" getMock.

My suggestion for the new one would be mock.

So you'd say

$this->mock('foo');

another option would be to slap another parameter on getMock and make a MockBuilderFunction allowCreationOfNonExistingClasss or something.

Maybe even have a release that only has a warning telling you

"You are mocking a non existing class. This might be not what you wanted. If this is in intentional behavior please use the $allowNonExisting flag."

Having a new function would allow for more cleanup but require a lot more work/docs/etc

Collaborator

edorian commented Nov 24, 2012

I'd like to do something about this for 3.8.

One option would be to have a new method that only mocks existing things and one to create new stuff and do proper error reporting there "deprecating" getMock.

My suggestion for the new one would be mock.

So you'd say

$this->mock('foo');

another option would be to slap another parameter on getMock and make a MockBuilderFunction allowCreationOfNonExistingClasss or something.

Maybe even have a release that only has a warning telling you

"You are mocking a non existing class. This might be not what you wanted. If this is in intentional behavior please use the $allowNonExisting flag."

Having a new function would allow for more cleanup but require a lot more work/docs/etc

@bartfeenstra

This comment has been minimized.

Show comment
Hide comment
@bartfeenstra

bartfeenstra Oct 28, 2013

What does the documentation say for stubbing/mocking things that don't already exist? Does it say "don't do this"? Because if that's so, we should be able to make 3.8 throw exceptions when this happens. If this breaks BC for people 1) they did something wrong in the first place and we just never told them until 3.8 2) they're tests anyway.

What does the documentation say for stubbing/mocking things that don't already exist? Does it say "don't do this"? Because if that's so, we should be able to make 3.8 throw exceptions when this happens. If this breaks BC for people 1) they did something wrong in the first place and we just never told them until 3.8 2) they're tests anyway.

@whatthejeff

This comment has been minimized.

Show comment
Hide comment
@whatthejeff

whatthejeff Oct 29, 2013

Collaborator

@bartfeenstra I don't think the documentation says anything about stubbing/mocking classes/interfaces/methods that don't exist.

Collaborator

whatthejeff commented Oct 29, 2013

@bartfeenstra I don't think the documentation says anything about stubbing/mocking classes/interfaces/methods that don't exist.

@1emming 1emming referenced this issue in silexphp/Silex Jun 27, 2014

Merged

remove not needed logger tests #967

fabpot added a commit to silexphp/Silex that referenced this issue Jun 30, 2014

minor #967 remove not needed logger tests (1emming)
This PR was merged into the 2.0.x-dev branch.

Discussion
----------

remove not needed logger tests

The `isset` checks on `$app['logger']` make no sense since `Application` always sets the logger (to `null`). Checks for `!empty` would be more safe/complete (altough `!empty && instance_of('LoggerInterface')` would be more ideal in some cases).
Updating the unit tests is more complicated because some code under test depends on classes outside of Silex.
Hacking the `class_exists` function or use `mocking` of classes that do not exists (@see sebastianbergmann/phpunit-mock-objects#12) would effect the whole test suit (as @stof pointed out).

Commits
-------

784a246 remove not needed logger tests

@whatthejeff whatthejeff referenced this issue in sebastianbergmann/phpunit Sep 8, 2014

Closed

Using method(...) with non-existent methods #1429

@rr-

This comment has been minimized.

Show comment
Hide comment

rr- commented Sep 8, 2014

👍

@rr-

This comment has been minimized.

Show comment
Hide comment
@Tobion

This comment has been minimized.

Show comment
Hide comment
@Tobion

Tobion Jun 23, 2016

@sebastianbergmann considering #296 has been implemented I think this one should be implemented as well. Then it is consistent that you get a warning when trying to mock a non-existing method or class.
That non-existing classes are created on-the-fly as empty classes without any method is alot of magic that hides potential problems.

Tobion commented Jun 23, 2016

@sebastianbergmann considering #296 has been implemented I think this one should be implemented as well. Then it is consistent that you get a warning when trying to mock a non-existing method or class.
That non-existing classes are created on-the-fly as empty classes without any method is alot of magic that hides potential problems.

@sebastianbergmann

This comment has been minimized.

Show comment
Hide comment
@sebastianbergmann

sebastianbergmann Jun 23, 2016

Owner

This has been taken care of in PHPUnit 5.4. Just use createMock().

This has been taken care of in PHPUnit 5.4. Just use createMock().

@Tobion

This comment has been minimized.

Show comment
Hide comment
@Tobion

Tobion Jun 23, 2016

@sebastianbergmann this is not correct. You can still create mocks of non-existing classes:

$mock= $this->createMock('NonExistingClass'); will not create a warning or error.

Tobion commented Jun 23, 2016

@sebastianbergmann this is not correct. You can still create mocks of non-existing classes:

$mock= $this->createMock('NonExistingClass'); will not create a warning or error.

@Tobion

This comment has been minimized.

Show comment
Hide comment
@Tobion

Tobion Jun 23, 2016

Nvm, I used a class that was magically defined with getMockBuilder before which of course succeded.

Tobion commented Jun 23, 2016

Nvm, I used a class that was magically defined with getMockBuilder before which of course succeded.

@Tobion

This comment has been minimized.

Show comment
Hide comment
@Tobion

Tobion Jun 23, 2016

I've found sebastianbergmann/phpunit#2228 which is really bad IMO.

Tobion commented Jun 23, 2016

I've found sebastianbergmann/phpunit#2228 which is really bad IMO.

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