Open owenconti opened 3 years ago
Yup this is a must. I like the general interface you've lined out here.
I'll think on it, and also take a look at the Laravel Event and Bus fakes to see if there's something I can leverage there. I might be able to knock something out here in the next day or two, depending on how much the new kiddos sleep 😅
No worries, I have a proof of concept on a fork that I can use in the meantime. Just working on one last piece and then I'll link it here.
That would be wonderful!
Okay here it is: https://github.com/owenconti/sidecar/pull/1/files
Everything is new code except for the isError
check. The mocked AWS response doesn't include the FunctionError
key (I may be doing something wrong), which was causing the previous isError
check to return true when it should have been false.
You'd probably want to add a dedicated method for asserting an async execution vs a sync execution too.
This is amazing. Thank you for doing this!
No worries, hopefully it helps some!
Since Sidecar
extends Facade
, it has the following functionality added which (imo) is a more useful mock than what you have here without any new code:
use Hammerstone\Sidecar\Sidecar;
// set the expectation in the test code
Sidecar::shouldReceive('execute')
->once()
->with(SomeFunction::class, ['foo' => 'bar'])
->andReturn('results');
// execute some runtime code that calls this internally
SomeFunction::execute(['foo' => 'bar']);
https://laravel.com/docs/9.x/mocking#mocking-facades
Your method is also adding testing code in runtime classes. Laravel's strategy with fakes is to encapsulate the testing code in a separate fake class and swap them out in the fake method. This ensures that the testing code affects the runtime code as little as possible.
Here's code for the Mail fake method as an example: https://github.com/laravel/framework/blob/9.x/src/Illuminate/Support/Facades/Mail.php#L45
But I don't believe it's necessary to create a fake class for just recording method executions. The mocking illustrated above does that already. Also read further in the docs about Facade spies.
Since
Sidecar
extendsFacade
, it has the following functionality added which (imo) is a more useful mock than what you have here without any new code:use Hammerstone\Sidecar\Sidecar; // set the expectation in the test code Sidecar::shouldReceive('execute') ->once() ->with(SomeFunction::class, ['foo' => 'bar']) ->andReturn('results'); // execute some runtime code that calls this internally SomeFunction::execute(['foo' => 'bar']);
https://laravel.com/docs/9.x/mocking#mocking-facades
Your method is also adding testing code in runtime classes. Laravel's strategy with fakes is to encapsulate the testing code in a separate fake class and swap them out in the fake method. This ensures that the testing code affects the runtime code as little as possible.
Here's code for the Mail fake method as an example: https://github.com/laravel/framework/blob/9.x/src/Illuminate/Support/Facades/Mail.php#L45
But I don't believe it's necessary to create a fake class for just recording method executions. The mocking illustrated above does that already. Also read further in the docs about Facade spies.
I tried to do it as you indicated as I think it looks great, but I couldn't do it, I get an error about an incorrect number of parameters.
No matching handler found for Mockery_3_Hammerstone_Sidecar_Manager::execute('App\Sidecar\StorageDownloadZipped', ['files' => [...], 'tenant' => 'the-id-tenant'], false, 'RequestResponse'). Either the method was unexpected or its arguments matched no expected argument list for this method
@mreduar
I tried to do it as you indicated as I think it looks great, but I couldn't do it, I get an error about an incorrect number of parameters.
If you look at the error you can see sidecar is adding two additional parameters to the execute call, false and 'RequestResponse'. Setting up my test like this worked as described:
Sidecar::shouldReceive('execute')
->once()
->with(MySidecarFunction::class, [
'expected' => 'params',
], false, 'RequestResponse')
->andReturn('some result');
It would be awesome if the package supported the ability to fake/mock a Sidecar execution, ie:
The idea here would be to keep the faking pattern similar to the Laravel first-party fakes (Http, Queue, etc):
fake()
with a mocked response for the lambda