Can't mock an object resolved with some constructor arguments

[osteel]

• Laravel Version: 8.44.0
• PHP Version: 8.0.2
• Database Driver & Version: MySQL 8.0

Description:

When resolving an object with constructor parameters using the service container, it seems that subsequently trying to mock that object fails.

Steps To Reproduce:

Say I've got an interface FooContract, with a bar method:

<?php

namespace App;

interface FooContract
{
    public function bar(): string;
}

And a class Foo implementing that interface, with a constructor expecting a $baz string, and the bar method returning 'qux':

<?php

namespace App;

class Foo implements FooContract
{
    public function __construct(public string $baz)
    {
    }

    public function bar(): string
    {
        return 'qux';
    }
}

And I bind FooContract to Foo in AppServiceProvider:

public function register()
{
    $this->app->bind(FooContract::class, Foo::class);
}

Say I write a simple test.

Using the service container to resolve FooContract works as expected:

public function testFoo()
{
    resolve(FooContract::class, ['baz' => 'baz'])->bar(); // "qux"
}

But then if I create a mock and bind it to FooContract, the mock seems to be ignored:

public function testFoo()
{
    $this->mock(FooContract::class, function (MockInterface $mock) {
        $mock->shouldReceive('bar')->once()->andReturn('corge');
    });

    resolve(FooContract::class, ['baz' => 'baz'])->bar(); // still "qux"
}

Without changing the test, and simply by removing the arguments when resolving FooContract using the service container, the mock is not ignored anymore:

public function testFoo()
{
    $this->mock(FooContract::class, function (MockInterface $mock) {
        $mock->shouldReceive('bar')->once()->andReturn('corge');
    });

    //resolve(FooContract::class, ['baz' => 'baz'])->bar();
    resolve(FooContract::class)->bar(); // "corge"
}

I would expect the bind to work whether there are constructor parameters or not.

[driesvints]

Heya, thanks for reporting.

I'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue and share the repository here? Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up.

laravel new bug-report --github="--public"

After you've posted the repository, I'll try to reproduce the issue.

Thanks!

[osteel]

Hi @driesvints, I'm having trouble with the bug-report codebase for some reason (e.g. the $this->mock() method doesn't seem to be recognised from test classes, and binding resolution seems to fail) but here is the repository, with a couple of instructions in the README file.

Let me know if you need anything else.

[driesvints]

Hey @osteel. It seems you committed all your code as a single commit. I need a separate commit with your custom changes to see what you've modified.

Edit: normally when you use the above command I gave it'll create the setup fresh laravel app separately. Did you amend your changes? (please don't do that)

[osteel]

@driesvints yeah I did, my bad. Also found the issue, now fixed. Let me just redo this, one sec

[osteel]

@driesvints there you go

[driesvints]

I have to throw in the towel here. I just can't figure it out. When I debug the container it returns the correct mocked instance but it arrives as the actual concrete implementation in the test somehow? No idea why that happens...

[osteel]

@driesvints well, in a way it's comforting that I wasn't just doing something stupid 😄

Thanks for looking into this – hopefully some reinforcements will help crack the case

[donnysim]

Container.php:740

if (isset($this->instances[$abstract]) && ! $needsContextualBuild) {
    return $this->instances[$abstract];
}

My 2 cents here, it is determined as $needsContextualBuild so it skips returning the registered instance (which is the mock) and builds it again, thus receiving the original concrete instead of the mocked one. I'm not sure what the purpose of the contextual build is so can't really tell what the correct way to fix this should be.

[driesvints]

@donnysim I checked into that piece of code and it did return the mocked instance for me. It just comes out as the concrete implementation. I have no idea why that is.

[donnysim]

@driesvints interesting, for me it didn't - $needsContextualBuild is true and it skips that part, if I remove the context check, the mocked instance is returned in the test and it passes.

[driesvints]

        if (isset($this->instances[$abstract]) && ! $needsContextualBuild) {
            if ($abstract === \App\FooContract::class) dd($this->instances[$abstract]);

            return $this->instances[$abstract];
        }

Gives

$ phpunit tests/Unit                                                                                                                                                                                                                ~/Sites/test-mocking
PHPUnit 9.5.5 by Sebastian Bergmann and contributors.

Mockery_0_App_FooContract {#432
  #_mockery_expectations: array:1 [
    "bar" => Mockery\ExpectationDirector {#433
      #_name: "bar"
      #_mock: Mockery_0_App_FooContract {#432}

...

[donnysim]

But that is triggered on

// Would expect to get "corge"...
$this->mock(FooContract::class, function (MockInterface $mock) {
    $mock->shouldReceive('bar')->once()->andReturn('corge');
});

when it calls

$result = resolve(FooContract::class, ['baz' => 'baz'])->bar(); // ... but still getting "qux"

it does not trigger that code, I presume because ['baz' => 'baz'] was passed and it assumes it needs to create a separate instance?

[driesvints]

Thanks @donnysim, that was helpful. I didn't realize the container also resolved the mocked dependency immediately.

I currently got it working with:

        if (isset($this->instances[$abstract]) &&
            ($this->instances[$abstract] instanceof MockInterface || ! $needsContextualBuild)) {
            return $this->instances[$abstract];
        }

But that wires the MockInterface to the container and I'm not sure that's wanted.

[Nacoma]

replicating the issue in the service provider (no mock)

In the non-working version - a concrete FooContract is registered as an instance in the container. This action does not replace the original binding.

The documentation does state that:

The given instance will always be returned on subsequent calls into the container

but instances are not registered as actual singletons. A new instance will be returned if it's explicitly requested (eg telling it to use new constructor arguments).

working version

Replacing the call to instance with an explicit singleton binding here should fix this.

If we do expect a mocked instance to be bound as a singleton then we simply need to register it as one.

[driesvints]

We're reverting the PR that fixed this because it's breaking other people's test suites. We'll need to look at an alternative approach.

@donnysim can you please try out my approach from above?

[donnysim]

@driesvints can't vouch for other use cases but it does work for us - both the contract and the concrete, including aliases is received as mocked instance despite the provided parameters.

[donnysim]

For some reason I just can't stop the feeling that mocking all instances is just wrong, like this will lead yet again to other issues like resolving multiples of the same contract but with different parameters leave you with the same mock. This would result once restrictions to fail even though they might be instantiated in 2 different methods. Somehow I'd want to say that an instance resolve callback should be the way to go to decide if you want to return a mocked instance or a real one, maybe something in the line of:

$this->mockInstance(FooContract::class, function (FooContract $instance, MockInterface $mock) {
    if ($something) {
        return $instance;
    }

    return $mock->shouldReceive('bar')->once()->andReturn('corge');
});

but not sure if I'm way off the target or not and if it's even feasable.

[sentiasa]

I have had the same issue - reverted back to "laravel/framework": "8.49.0" and the mocking works again.

[driesvints]

@donnysim I'm going to close this now. Feel free to reply when you've found time to test that out.

[osteel]

Hey,

I couldn't find a way to solve this without changing Container.php so I went for an approach inspired by @Nacoma's comments and PR. Registering the mock as a singleton solves the issue so I opened a PR adding a method to the InteractsWithContainer trait to make that easier.

It's all new code so won't affect existing test suites.

[osteel]

PR was closed 🤷

[supun-io]

Still have this issue on Laravel 9.14.1. @osteel Found any workarounds?

Edit: I found a solution here. But, it would be nice if $this->mock could be used.

[SilvaQ]

same issue laravel 9 @osteel

[jameshulse]

Same issue in Laravel 11 all this time later. Would love to know if there is a resolution as it can be a real time sink trying to debug why a mock isn't working.

The issue can be resolved manually by changing:

$this->mock(MyClass::class, ...);

To:

$this->app->singleton(MyClass::class, function () {
    $mock = $this->mock(MyClass::class)
    ...setup mock
    return $mock;
});

But this obviously isn't very discoverable, or ideal. Hopefully we can come to some resolution between Laravel tests, and the container.

[jobyh]

+1 Also experiencing this issue. Hey @osteel 👋 fancy meeting you here! 😄