sidecar icon indicating copy to clipboard operation
sidecar copied to clipboard

Support for testing/mocking

Open owenconti opened this issue 3 years ago • 9 comments

It would be awesome if the package supported the ability to fake/mock a Sidecar execution, ie:

/** @test */
public function it_calls_sidecar_function()
{
    // Given
    $mockedResponse = [
        'hello' => 'world'
    ];
    SomeFunction::fake($mockedResponse);

    // When
    $this->get('/call-function')->assertSuccessful();

    // Then
    $expectedParams = [
        'foo' => 'bar'
    ];
    SomeFunction::assertExecuted($expectedParams);
}

The idea here would be to keep the faking pattern similar to the Laravel first-party fakes (Http, Queue, etc):

  • You call fake() with a mocked response for the lambda
  • You call your controller/command/job, etc that will execute the lambda
  • You call an assertion method to assert that the lambda was invoked with the given set of parameters

owenconti avatar Jun 13 '21 16:06 owenconti

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 😅

aarondfrancis avatar Jun 13 '21 17:06 aarondfrancis

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.

owenconti avatar Jun 13 '21 17:06 owenconti

That would be wonderful!

aarondfrancis avatar Jun 13 '21 17:06 aarondfrancis

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.

owenconti avatar Jun 13 '21 17:06 owenconti

This is amazing. Thank you for doing this!

aarondfrancis avatar Jun 13 '21 20:06 aarondfrancis

No worries, hopefully it helps some!

owenconti avatar Jun 13 '21 20:06 owenconti

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.

datashaman avatar Apr 05 '22 05:04 datashaman

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.

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 avatar Jun 21 '22 23:06 mreduar

@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');

benbjurstrom avatar Aug 23 '22 16:08 benbjurstrom