Implement `Closure` dispatcher. #24

shulard · 2015-11-17T09:24:09Z

Hello,

I've tried to work on the #15 issue. The code is moved from the Basic to a new Closure dispatcher. I've also added a test suite based on the ClassMethod one.

I'm not totally fine because there is a lot of code duplication between the ClassMethod, the Closure and also in the Closure... It's a draft which work, I think ready for review 😄

Hywan · 2015-11-17T09:26:34Z

Closure.php

+        return $return;
+    }
+
+    protected function populateVariables(&$variables, $rtv)


Add API documentation please.

Hywan · 2015-11-17T09:27:53Z

This is a draft and I made a very very quick/high review. Please, tell me when a deeper review is required.

shulard · 2015-11-17T09:37:24Z

For sure it is a draft, thanks for the review... I'll make it cleaner quickly with a better atoum integration.

shulard · 2015-11-17T10:15:32Z

@Hywan just one question about function mock... I tried to use it but I don't understand how to... I tried to found some doc in atoum but there is only native function mocks...

Here my code sample :

$this->function->dispatchedMethod = function($test, $foo, $bar) {
  $test
    ->string($foo)
      ->isEqualTo('foo')
    ->string($bar)
      ->isEqualTo('bar');
};

I tried to call the dispatchedMethod but it is not defined. Are the mocks only useful here to generate a closure from a function ?
I also tried to add my function definition in the given call but nothing better...

Is there any example of that kind of use ?

shulard · 2015-11-23T11:12:26Z

Hello @Hywan I need more help about that test cases,

I tried different things with function mocks but can't get it works in my case...

As an example, I write this in a test case :

$this
    ->given(
        $this->function->dispatchedMethod = function($test, $foo, $bar) {
            $test
                ->string($foo)
                    ->isEqualTo('foo')
                ->string($bar)
                    ->isEqualTo('bar');
        },
        $router = new Router\Cli(),
        $router->get(
            'a',
            '(?<_call>dispatchedMethod) ' .
            '(?<foo>foo) ' .
            '(?<bar>bar)'
        ),
        $this->route(
            $router,
            'dispatchedMethod foo bar'
        ),

        $dispatcher = new LUT\Closure(),
        $dispatcher->getParameters()->setParameters([
            'synchronous.call' => '(:call:U:)',
            'synchronous.able' => '(:able:)'
        ])
    )
    ->when($dispatcher->dispatch($router));

I'm sure that this code define the function mock for dispatchedMethod but this function is not present at the root namespace and can't be found by the Dispatcher...

It's more about atoum behaviour than my PR, but I'm sure that you know atoum better than me 😄

Hywan · 2015-11-24T08:10:36Z

Sorry for the late reply. Please, can you explain what do you want to test and what your test case does?

Hywan · 2015-11-24T08:10:58Z

I am assigning @osaris since I have too much work.

shulard · 2015-11-24T08:51:38Z

Hello @Hywan,

Don't worry for the delay, I was just thinking about this PR yesterday and I've added a comment 😄

shulard · 2015-11-24T08:52:16Z

Here the requested details on my test case which is built to test the behaviour of the closure dispatcher :

As a predicate :
- Register a function that will be used in the test - dispatchedMethod
- Define some routing config (basically the router must call my function with 2 parameters)
Then execute the test with a $dispatcher->dispatch() call
For the oracle, the registered function contains the code with assertions

I've took the logic from the ClassMethod dispatcher which maybe is not the good example to start...

My main problem here is that my registered function is not registered in the place I need :

$this->function->dispatchedMethod is valid
__NAMESPACE__."\dispatchedMethod" does not exists
"dispatchedMethod" does not exists

So I don't know how to use my mocked function to deal with my test needs...

shulard · 2015-12-02T09:23:59Z

Hello, any news about this PR ? I'm sorry but I'm lost in the "atoum" features...

Hywan · 2015-12-02T09:24:44Z

ping @osaris :-)

shulard · 2016-02-01T20:25:05Z

Hello @Jir4, do you know atoum function mock enough to help me here ?

Jir4 · 2016-02-01T22:22:09Z

I'm not aware about atoum :/ I don't use it at all.

Hywan · 2016-02-02T07:57:21Z

@shulard $this->function->foo = function () { … } :-).

shulard · 2016-02-02T11:42:30Z

@Hywan, so my code is right here...

I'm already able to create the mock function but I can't use it through my dispatcher.

In my test case, I try to route to the mocked function with parameters. The router is not aware of the Test\Unit\Closure test mocked functions... Maybe for that case it's not possible to use mocked function ?

Hywan · 2016-02-02T14:17:50Z

Are you trying to mock a function or a method? If this is a method, then:

$this->calling($object)->$method = function (…) { … }

shulard · 2016-02-02T14:28:37Z

I try to mock a function to test the Closure dispatcher with a function name. The function can be namespaced but it's a simple function.

Hywan · 2016-02-02T14:39:39Z

@shulard Show me what piece of code you are trying to test please :-).

shulard · 2016-02-02T14:49:27Z

@Hywan: Of course 😄

class Closure extends Test\Unit\Suite
{
    public function case_function_name_in_rule_pattern()
    {
        $this
            ->given(
                $this->function->dispatchedMethod = function($test, $foo, $bar) {
                    $test
                        ->string($foo)
                            ->isEqualTo('foo')
                        ->string($bar)
                            ->isEqualTo('bar');
                },
                $router = new Router\Cli(),
                $router->get(
                    'a',
                    '(?<_call>dispatchedMethod) ' .
                    '(?<foo>foo) ' .
                    '(?<bar>bar)'
                ),
                $this->route(
                    $router,
                    'dispatchedMethod foo bar'
                ),

                $dispatcher = new LUT\Closure(),
                $dispatcher->getParameters()->setParameters([
                    'synchronous.call' => '(:call:U:)',
                    'synchronous.able' => '(:able:)'
                ])
            )
            ->when($dispatcher->dispatch($router));
    }

    protected function route(Router $router, $uri, array $extraVariables = [])
    {
        $router->route($uri);
        $theRule                                  = &$router->getTheRule();
        $theRule[$router::RULE_VARIABLES]['test'] = $this;

        foreach ($extraVariables as $name => $value) {
            $theRule[$router::RULE_VARIABLES][$name] = $value;
        }

        return $router;
    }
}

With that code I just get the atoum error: Hoa\Dispatcher\Closure::resolve(): (0) Function dispatchedMethod is not found.

I mock the method and try to use it in the dispatcher. Before using the mocked function, I've created some namespaced functions in my test file... It works but was not really clean...

An example here

Hywan · 2016-02-02T15:08:54Z

Can you show me the SUT (the method you are trying to test) please? I don't understand why you're mocking your own function.

shulard · 2016-02-02T15:12:34Z

I'm trying to test the Closure::resolve method: https://github.com/hoaproject/Dispatcher/pull/24/files#diff-d4e09e011fe8d15e008ce4ef76685e74R62

This method must be able to dispatch to a Closure or a function. With this test case I try to validate that the Closure::resolve is able to dispatch to a function.

Am I missing the whole thing ? Maybe my testing strategy is not the good one ?

Hywan · 2016-02-02T15:15:04Z

So you don't need to mock any thing. You have a function that will be called by the dispatcher.

shulard · 2016-02-02T15:25:54Z

Yes but I need to create that function somewhere...

Maybe I can use a PHP function instead of creating a useless one. Then validate the result of the call.

Hywan · 2016-02-03T06:08:57Z

@shulard You can use your own function, of course. To call it, don't forget the namespace (__NAMESPACE__ . '\dispatchedMethod').

shulard · 2016-02-03T10:25:26Z

Super 👍
I'll work on the test suite quickly !

shulard · 2016-02-19T17:09:35Z

Hello @Hywan,

I've rebased my branch on master and update the test suite to avoid useless function creation.

Hywan · 2016-02-22T07:34:28Z

Is it ready for a review?

shulard · 2016-02-22T09:28:26Z

Yes I think !
I hope that I've used Atoum in a better way :)

I think we'll need a little refactoring because there is duplication between ClassMethod and Closure dispatchers...

Hywan · 2016-03-23T06:09:40Z

Looks correct, but I am waiting on a first review from @osaris :-)!

osaris · 2016-05-14T16:59:42Z

Hey guys, I'm here :) It looks good for me too ! Thanks for testing your feature.

shulard · 2016-05-31T12:15:15Z

Related to #11

Hywan · 2016-05-31T12:15:44Z

Will merge it as soon as possible.

shulard · 2016-12-13T14:44:18Z

Hello @Hywan,

Any news about that PR ?
I just rebased all the commits on master...

Also refactor some code duplication around `Kit

Adaptation of the `ClassMethod` test suite.

Hywan added enhancement in progress difficulty: medium labels Nov 17, 2015

Hywan self-assigned this Nov 17, 2015

Hywan reviewed Nov 17, 2015
View reviewed changes

Closure.php

return $return;

}

protected function populateVariables(&$variables, $rtv)

Copy link

Member

Hywan Nov 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add API documentation please.

Hywan assigned osaris and unassigned Hywan Nov 24, 2015

shulard force-pushed the feature/issue-15 branch from 34963f3 to 894848c Compare February 19, 2016 17:08

shulard added 4 commits December 13, 2016 15:44

Move Basic code to the new Closuredispatcher

442087b

Also refactor some code duplication around `Kit

Add a unit test suite for the Closure dispatcher.

89efbba

Adaptation of the `ClassMethod` test suite.

Fix CS

24e04aa

Use the hoa/consistency package

77170fd

shulard force-pushed the feature/issue-15 branch from 894848c to 77170fd Compare December 13, 2016 14:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `Closure` dispatcher. #24

Implement `Closure` dispatcher. #24

shulard commented Nov 17, 2015

Hywan Nov 17, 2015

Hywan commented Nov 17, 2015

shulard commented Nov 17, 2015

shulard commented Nov 17, 2015

shulard commented Nov 23, 2015

Hywan commented Nov 24, 2015

Hywan commented Nov 24, 2015

shulard commented Nov 24, 2015

shulard commented Nov 24, 2015

shulard commented Dec 2, 2015

Hywan commented Dec 2, 2015

shulard commented Feb 1, 2016

Jir4 commented Feb 1, 2016

Hywan commented Feb 2, 2016

shulard commented Feb 2, 2016

Hywan commented Feb 2, 2016

shulard commented Feb 2, 2016

Hywan commented Feb 2, 2016

shulard commented Feb 2, 2016

Hywan commented Feb 2, 2016

shulard commented Feb 2, 2016

Hywan commented Feb 2, 2016

shulard commented Feb 2, 2016

Hywan commented Feb 3, 2016

shulard commented Feb 3, 2016

shulard commented Feb 19, 2016

Hywan commented Feb 22, 2016

shulard commented Feb 22, 2016

Hywan commented Mar 23, 2016

osaris commented May 14, 2016

shulard commented May 31, 2016

Hywan commented May 31, 2016

shulard commented Dec 13, 2016 •

edited

Loading

Implement Closure dispatcher. #24

Are you sure you want to change the base?

Implement Closure dispatcher. #24

Conversation

shulard commented Nov 17, 2015

Hywan Nov 17, 2015

Choose a reason for hiding this comment

Hywan commented Nov 17, 2015

shulard commented Nov 17, 2015

shulard commented Nov 17, 2015

shulard commented Nov 23, 2015

Hywan commented Nov 24, 2015

Hywan commented Nov 24, 2015

shulard commented Nov 24, 2015

shulard commented Nov 24, 2015

shulard commented Dec 2, 2015

Hywan commented Dec 2, 2015

shulard commented Feb 1, 2016

Jir4 commented Feb 1, 2016

Hywan commented Feb 2, 2016

shulard commented Feb 2, 2016

Hywan commented Feb 2, 2016

shulard commented Feb 2, 2016

Hywan commented Feb 2, 2016

shulard commented Feb 2, 2016

Hywan commented Feb 2, 2016

shulard commented Feb 2, 2016

Hywan commented Feb 2, 2016

shulard commented Feb 2, 2016

Hywan commented Feb 3, 2016

shulard commented Feb 3, 2016

shulard commented Feb 19, 2016

Hywan commented Feb 22, 2016

shulard commented Feb 22, 2016

Hywan commented Mar 23, 2016

osaris commented May 14, 2016

shulard commented May 31, 2016

Hywan commented May 31, 2016

shulard commented Dec 13, 2016 • edited Loading

Implement `Closure` dispatcher. #24

Implement `Closure` dispatcher. #24

shulard commented Dec 13, 2016 •

edited

Loading