Framework: [5.4] Unable to mock Schema

[markdwhite]

• Laravel Version: 5.4
• PHP Version: 7.0.8
• Database Driver & Version: php7.0-sqlite3

Description:

As detailed in previous ticket https://github.com/laravel/framework/issues/7661, and seems to apply to through all versions to current:

Support\Facades\Schema::getFacadeAccessor() returns an instance of Builder.
Support\Facades\Facade::getFacadeAccessor() phpdocs state that a string is expected
Support\Facades\Facade::isMock() tries to use the return of getFacadeAccessor() as an array index, which fails when an object is returned.

Either Schema can be mocked and the above is a bug, or Schema is not intended to be mocked and Schema::isMock() should override to throw an Exception with a suitable message (possibly).

Steps To Reproduce:

Clone master
Install composer dependencies
Edit tests\ExampleTest.php and add:
public function testSchemaMockFails()
{
Schema::shouldReceive('test')->andReturn(true);
}

Run phpunit with output:
1) ExampleTest::testSchemaMockFails
ErrorException: Illegal offset type in isset or empty

[abinadi]

I am also experiencing the same issue. Is the Schema facade not supposed to be mocked?

[GrahamCampbell]

I don't see the use case for mocking that facade. You're not really testing anything, you're just writing out your code again.

[GrahamCampbell]

More useful tests would inspect what it actually did to the database.

[abinadi]

Either Schema can be mocked and the above is a bug, or Schema is not intended to be mocked and Schema::isMock() should override to throw an Exception with a suitable message (possibly).

@markdwhite makes a good point. Perhaps you could address it, @GrahamCampbell

[markdwhite]

I agree that we are probably talking edgey use-cases here: with the 2 affected users here and those few in the original thread, it's not having a wide-ranging effect But it still smells like a bug.

My use case concerns the creation of a number of engine=MEMORY tables that are updated regularly throughout the day and utilised for a short period of time for hundreds of thousands of queries where speed of response is paramount. Hence the MEMORY table. If mySQL is restarted for any reason (security update), the tables are lost and must be recreated. It makes sense to do this from within the Command that updates the tables rather than manually, so currently I try{} to Schema::create() and catch{} with a filter for an error indicating that the table exists then proceed to update the table. I can test this by creating a table using migrations which causes the "table already exists" exception to be thrown, but cannot test where a different exception is thrown without deliberately introducing code that throws the exception just to catch it. Which is ugly. So I need that Schema::shouldReceive('create')->andThrow(...). But I only need it for test completeness and can live with @codeCoverageIgnore if Schema cannot be mocked.

I can't test the effect on the database in this example because I'm testing all possible courses of action after the caught exception, not that Schema is doing what it should, and there is also the view that unit tests and integration tests are not the same and should be testable separately.

As I've said before, if it can be mocked - it's broken and I don't get the impression that Laravel is about leaving broken code in place. If it can't be mocked, maybe that should be unambiguous, and I've suggested that Schema::isMock() should throw an exception to that effect, but readily accept better suggestions.

[Dylan-DPC]

This issue can be closed

[laurencei]

Hi there,

Welcome to Laravel and we are glad to have you as part of the community.

Unfortunately this GitHub area is not for ideas, suggestions etc. This is only for issues/bugs with the framework code itself.

I will be closing your ticket here. You are able to open a ticket at https://github.com/laravel/ideas

Alternatively you are able to open a PR using the Contributions guidelines: https://laravel.com/docs/5.7/contributions#which-branch

If you feel I've closed this issue in error, please provide more information about how this is a framework issue, and I'll reopen the ticket.

Thanks in advance.

[SlayerBirden]

More useful tests would inspect what it actually did to the database.

This statement just shows that you don't understand what Unit tests are for. The whole "mocking" functionality exists specifically to simulate the interactions between objects and systems. If it's hard for you to support the same functionality for all Facades just add a note somewhere in the docs listing which ones do not support Mocking. Plainly closing valid bug seems odd to me.

@GrahamCampbell ^