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

app helper doesn't return mocked instance when parameters are present #19450

Closed
dallincoons opened this issue Jun 2, 2017 · 15 comments
Closed

Comments

@dallincoons
Copy link
Contributor

  • Laravel Version: 5.4.24
  • PHP Version: 7.0.15

When binding a mock instance to a class, resolving that class acts differently depending on whether parameters are passed in:

$mock = \Mockery::mock(SomeClass::class);
app()->instance(SomeClass::class, $mock);

get_class(app(SomeClass::class)) yields Mockery_0_SomeClass which is the mockery instance

when parameters are passed in:
get_class(app(SomeClass::class, ['param' => 'fakeparam'])); this yields SomeClass instead of the mockery instance.

This seems like an unfortunate inconsistency, but maybe this functionality is intentional?

@laurencei
Copy link
Contributor

In the original PR #18271 - Taylor says:

In this version, it will never use a previously resolved singleton instance when calling makeWith. In my opinion it should make a new instance each time this method is called because the given parameter array is dynamic.

I'm wondering if this is a side effect of that...

...but it does seem slightly broken - because it means it is not fullable testable.

@themsaid themsaid closed this as completed Jun 7, 2017
@themsaid themsaid reopened this Jun 7, 2017
@elynnaie
Copy link

I wonder then how I can mock a job that accepts parameters, because I currently cannot due to this issue.

@skovmand
Copy link

skovmand commented Sep 4, 2017

I have this problem too. The current behaviour seems like a bug to me. This issue helped me understand it.

So only way now in my project is to introduce a conditional for app()->environment() === 'testing'. As well as introduce default values for the parameters.

Please reconsider this one :-)

@driade
Copy link
Contributor

driade commented Feb 17, 2018

I had to do a dirty trick to test this too. Would love to see this changed someway.

@giginos
Copy link

giginos commented Jun 5, 2018

Are there any news? It is really exhausting writing tests with this behavior. I'm using version 5.5.

@b-ramin
Copy link

b-ramin commented Jun 27, 2018

It appears that passing $parameters in to the Container resolve method forces it to resolve contextually:

$needsContextualBuild = ! empty($parameters) || ! is_null(
    $this->getContextualConcrete($abstract)
);

Simply deleting ! empty($parameters) || seems to fix the problem for me. I'm not familiar enough with Laravels inner workings to be sure if that is the proper fix though, or if it breaks anything else.

@mfn
Copy link
Contributor

mfn commented Aug 3, 2018

See also #25041 for recent discussions.

@mikepmtl
Copy link

mikepmtl commented Jan 4, 2019

In case someone else is looking for this. You can mock it like this in your tests.
Use bind() instead of instance()

       $orders_mock = \Mockery::mock(OrdersRequest::class)->makePartial();
       $orders_mock->shouldReceive('send')->andReturn($order_response);
       $this->app->bind(OrdersRequest::class, function() use ($orders_mock){
           return $orders_mock;
       });

And it will use the Mock.

@apoca
Copy link

apoca commented Feb 24, 2019

@mikepmtl even when you call $this->json('POST',....) ?! Example:

`$request = [.....];

    $paypalPayment = Mockery::mock(
        PayPalPayment::class,
        [$request]
    )->makePartial();

    $object = (object)null;
    $object->status = 'APPROVED';
    $object->purchase_units[0]->amount['value'] = 15;
    $object->purchase_units[0]->amount['currency_code'] = 'EUR';

    $paypalPayment->shouldReceive('captureDetail')
        ->andReturn(json_decode(json_encode($object), true));

    $paypalPayment->shouldReceive('capturePayment')
        ->andReturn([
            'status' => 'COMPLETED',
        ]);

    $this->app->bind(PayPalPayment::class, function () use ($paypalPayment) {
        return $paypalPayment;
    });

    $this->json(
        'POST',
        "{$this->baseUrl}/{$this->version}/orders/pay",
        $request,
        [
            'Content-Type' => 'application/json',
            'Accept' => 'application/json',
        ]
    )->assertJsonFragment([
        'data' => [
            'method' => 'paypal',
            'status' => 'SUCCESS',
            'response' => [
                'status' => 'COMPLETED',
            ],
        ],
    ])->assertStatus(HttpStatusCodes::OK);`

@lk77
Copy link

lk77 commented May 10, 2019

Hello,

it seems that i have the same issue in 5.8.16,
in case someone find this issue via google like i did,
i solved this by doing :

App::offsetSet(Class::class, $this->mock(Class::class, function ($mock) {
    // do whatever you want            
    $mock;
}));

until this issue is resolved (if it's resolved at all)

@natsu90
Copy link

natsu90 commented Jan 22, 2020

I have kind of similar problem here, https://stackoverflow.com/q/59853235, but none of the solutions here works. Any idea?

@rytisder
Copy link

rytisder commented Jul 17, 2020

In case someone else is looking for this. You can mock it like this in your tests.
Use bind() instead of instance()

       $orders_mock = \Mockery::mock(OrdersRequest::class)->makePartial();
       $orders_mock->shouldReceive('send')->andReturn($order_response);
       $this->app->bind(OrdersRequest::class, function() use ($orders_mock){
           return $orders_mock;
       });

And it will use the Mock.

This worked just perfectly.

Just don't forget to instantiate object via App:make(YourObject::class, ['argument' => $argument]) instead of new YourObject($argument).

@trip-somers
Copy link

bind() also works as a replacement for $this->mock(). My comment from #25041:

I had been using $this->mock() in my tests, but it wasn't properly mocking an app(ClassToMock::class. ['param' => $param]) instance. I switched to Mockery::mock() + bind() and the mocking worked as expected.

@lk77
Copy link

lk77 commented Nov 14, 2023

Hello,

i'm having this issue again in 2023, the App::offsetSet trick from 2019 still works at least, but it would be great to have something like mockWith to be able to specity parameters

@opejovic
Copy link

In case someone else is looking for this. You can mock it like this in your tests. Use bind() instead of instance()

       $orders_mock = \Mockery::mock(OrdersRequest::class)->makePartial();
       $orders_mock->shouldReceive('send')->andReturn($order_response);
       $this->app->bind(OrdersRequest::class, function() use ($orders_mock){
           return $orders_mock;
       });

And it will use the Mock.

This solved my issue.

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

No branches or pull requests