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

QueryBus testing #157

Closed
lezhnev74 opened this issue Apr 21, 2017 · 9 comments
Closed

QueryBus testing #157

lezhnev74 opened this issue Apr 21, 2017 · 9 comments
Labels

Comments

@lezhnev74
Copy link

lezhnev74 commented Apr 21, 2017

Hi!

I am currently finding my way around with QueryBus. And I got confused with testing it. I created an empty project, pulled up just a few things:

"require": {
        "prooph/service-bus": "^6.0",
        "react/promise": "^2.5"
    },
    "require-dev": {
        "phpunit/phpunit": "^6.1"
    }

and then run this test:

<?php
declare(strict_types=1);


use PHPUnit\Framework\TestCase;
use Prooph\ServiceBus\QueryBus;

final class QueryTest extends TestCase
{
    function test_query() {
    
        $queryBus = new QueryBus();
        $queryBus->dispatch('anyname')->then(function(){
            //will not get there anyway
        }, function(Throwable $e){
            // Does nothing
            $this->fail($e->getMessage());
        });
        
    
    }
}

And it is green.
I get control in second callable, but PHPUnit did not respect the fail() call at all.

Do you have any suggestions on how to test queries / how to fix that test?

P.s. I browsed through the proophessor-do but did not find query tests there.

@basz
Copy link
Member

basz commented Apr 21, 2017

This happens because you tell phpunit to expect an exception thrown, but what actually happens is that the promise returns an Exception instance, not a 'thrown' one...

however, since you would normally get the result of either method and do something with it I recommend this which works also;

    public function test_query()
    {
        $succesCalled = false;
        $failedCalled = false;

        $queryBus = new QueryBus();
        $queryBus->dispatch('anyname')->then(
            function () use (&$succesCalled) {
                $succesCalled = true;
            },
            function (Throwable $e) use (&$failedCalled) {
                $failedCalled = true;
            });

        $this->assertFalse($succesCalled);
        $this->assertTrue($failedCalled);
    }

or

    public function test_query()
    {
        $queryBus = new QueryBus();
        $queryBus->dispatch('anyname')->then(
            function ($response) use (&$result) {
                $result = $response;
            },
            function ($response) use (&$result) {
                $result = $response;
            });

        $this->assertInstanceOf(\Prooph\ServiceBus\Exception\RuntimeException::class, $result);
        $this->assertContains('Message dispatch failed. See previous exception for details.', $result->getMessage());
    }

@lezhnev74
Copy link
Author

lezhnev74 commented Apr 21, 2017

Thanks for the code samples! Your suggestions are clear.

I understand that this is more the issue from reactphp/promise package than from the Prooph.
I think it is worth mentioning this conversation on reactphp/promise page which led me to this done() function.

I managed to find another way to properly fail the test (by calling fail()) by adding done() call at the end:

<?php
declare(strict_types=1);

use PHPUnit\Framework\TestCase;
use Prooph\ServiceBus\QueryBus;

final class QueryTest extends TestCase
{
    function test_query()
    {
        
        $queryBus = new QueryBus();
        $queryBus->dispatch('anyname')
                 ->then(function () {
                     //....
                 }, function (Throwable $e) {
                     $this->fail($e->getMessage());
                 })
                 ->done(); //    <--- THIS IS THE KEY HERE
        
    }
}

@prolic
Copy link
Member

prolic commented Apr 21, 2017

Can this issue be closed now?

@basz
Copy link
Member

basz commented Apr 21, 2017

should we add that done() thing?

@prolic
Copy link
Member

prolic commented Apr 21, 2017

maybe add some documentation? PR's are welcome :)

@oqq
Copy link
Member

oqq commented Apr 25, 2017

We should refer to the promise package instead of document how promises works by our own.
I use always that simple rule:
Either return the promise or call done() on it.

@prolic prolic closed this as completed Sep 19, 2017
@lezhnev74
Copy link
Author

lezhnev74 commented Dec 24, 2017

Guys, can you take a look at this issue as well, as it is relatively the same to this issue.
prooph/laravel-package#24

Documentations says nothing about how to resolve Deferred object inside finder when FinalizeGuard plugin is enabled.

@lezhnev74
Copy link
Author

I managed to reproduce the error in a simple test:
https://github.com/lezhnev74/prooph-query-test/blob/prooph-bare/tests/QueryTest.php

Results:

  • executing query with no guards - pass
  • executing query with RouteGuard - pass
  • executing query with FinalizeGuard - fail

Looks like I follow exactly what @basz suggested

@basz
Copy link
Member

basz commented Dec 25, 2017

Open a new issue. Definitly a problem i would think

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

No branches or pull requests

4 participants