Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for testing/mocking #16

Open
owenconti opened this issue Jun 13, 2021 · 9 comments
Open

Support for testing/mocking #16

owenconti opened this issue Jun 13, 2021 · 9 comments
Labels
enhancement New feature or request

Comments

@owenconti
Copy link
Contributor

owenconti commented Jun 13, 2021

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
@aarondfrancis aarondfrancis added the enhancement New feature or request label Jun 13, 2021
@aarondfrancis
Copy link
Owner

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 😅

@owenconti
Copy link
Contributor Author

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.

@aarondfrancis
Copy link
Owner

That would be wonderful!

@owenconti
Copy link
Contributor Author

owenconti commented Jun 13, 2021

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.

@aarondfrancis
Copy link
Owner

This is amazing. Thank you for doing this!

@owenconti
Copy link
Contributor Author

No worries, hopefully it helps some!

@datashaman
Copy link
Contributor

datashaman commented Apr 5, 2022

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.

@mreduar
Copy link

mreduar commented Jun 21, 2022

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

@benbjurstrom
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants