laravel / ideas

Issues board used for Laravel internals discussions.
938 stars 28 forks source link

Better error handling for Mockery Exception in Feature Test #2528

Open tuanpt-0634 opened 3 years ago

tuanpt-0634 commented 3 years ago

Note: this is not bug report and I think it doesn't a feature request, so I create issue here. Sorry for any inconvenience.

Version

Description:

Currently, when do mocking in a Feature test, for example:

class ProductControllerTest extends TestCase
{
    protected $productServiceMock;

    public function setUp(): void
    {
        parent::setUp();

        $this->productServiceMock = $this->mock(ProductService::class);
    }

    public function test_it_return_back_after_submit_with_valid_input()
    {
        $productId = 123;
        $stubResult = 100;
        $this->productServiceMock->shouldReceive('calculate')
            ->once()
            ->with($productId)
            ->andReturn($stubResult);

        $url = action([ProductController::class, 'check']);
        $response = $this->post($url, ['product_id' => $productId]);

        $response->assertStatus(302);
        $response->assertSessionHas('status', $stubResult);
    }
}

If the mock expectation failed, because of default Exception handler, we will receive $response with status = 500 and PHPUnit output will be:

1) Tests\Feature\Http\Controllers\ProductControllerTest::test_it_return_back_after_submit_with_valid_input
Expected status code 302 but received 500.
Failed asserting that 302 is identical to 500.

/var/www/html/vendor/laravel/framework/src/Illuminate/Testing/TestResponse.php:186
/var/www/html/Tests/Feature/Http/Controllers/ProductControllerTest.php:24

It is hard to debug which causes test failed. Compare to mockery exception stack trace:

1) Tests\Feature\Http\Controllers\ProductControllerTest::test_it_return_back_after_submit_with_valid_input
Mockery\Exception\NoMatchingExpectationException: No matching handler found for Mockery_0_App_Services_ProductService::calculate(123). Either the method was unexpected or its arguments matched no expected argument list for this method

/var/www/html/vendor/mockery/mockery/library/Mockery/ExpectationDirector.php:92
/var/www/html/app/Http/Controllers/ProductController.php:43
/var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Controller.php:54
/var/www/html/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php:45

Solutions

There are serveral ways to improve this, but I am not sure should we integrate to framework. Please suggest, I can make a pull request.

Option 1: update default Exception hander:

namespace App\Exceptions;

use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
use Throwable;

class Handler extends ExceptionHandler
{
    public function render($request, Throwable $e)
    {
        if ($e instanceof \Mockery\Exception) {
            throw $e;
        }

        return parent::render($request, $e);
    }
}

Option 2: Update trait tests/CreatesApplication.php

namespace Tests;

use App\Exceptions\Handler;
use Illuminate\Contracts\Console\Kernel;
use Illuminate\Contracts\Debug\ExceptionHandler;

trait CreatesApplication
{
    /**
     * Creates the application.
     *
     * @return \Illuminate\Foundation\Application
     */
    public function createApplication()
    {
        $app = require __DIR__.'/../bootstrap/app.php';

        $app->make(Kernel::class)->bootstrap();

        $app->instance(ExceptionHandler::class, new class($app) extends Handler {
            public function render($request, \Throwable $e)
            {
                if ($e instanceof \Mockery\Exception) {
                    throw $e;
                }

                return parent::render($request, $e);
            }
        });

        return $app;
    }
}

Option 3: update Laravel defaut test case

vendor/laravel/framework/src/Illuminate/Foundation/Testing/TestCase.php And bind new exception hander in this class.

driesvints commented 3 years ago

Heya, thanks for submitting this.

This seems like a feature request or an improvement so I'm moving this to the ideas repository instead. It's best to post these in the ideas repository in the future to get support for your idea. After that you may send a PR to the framework. Please only use the laravel/framework issue tracker to report bugs and issues with the framework.

Thanks!

tuanpt-0634 commented 3 years ago

Thank you @driesvints

For this issue, I think option 2 is easiest way because Mockery is dependency of https://github.com/laravel/laravel Or integrate to framework to support several mock framework.

What do you think?