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

Possibility to make a Laravel module/helper ? #299

Closed
isimmons opened this issue Mar 1, 2014 · 23 comments
Closed

Possibility to make a Laravel module/helper ? #299

isimmons opened this issue Mar 1, 2014 · 23 comments

Comments

@isimmons
Copy link

isimmons commented Mar 1, 2014

I've been toying around with phpspec and Laravel.

I'm not actually experienced enough to know how to do this so let me just show what I've found in making phpspec work with Laravel.

Using phpspec.yml I can create specs that properly generate files in the right location so that is good.

Basic dependency injection works

function let($somedependency)
{
    $somedependency->beADoubleOf('SomeNameSpace\SomeDependency');
    $this->beConstructedWith($somedependency);
}

But with Laravel we use aliases or Facades and the reason these work in Laravel testing with PHPunit is because tests extend TestCase which creates an instance of the application. To do the same thing with phpspec I copied and modified Laravels TestCase and extended that from my spec

A sample spec

<?php

namespace spec\Controllers;

//use PhpSpec\ObjectBehavior;
use spec\SpecCase;
use Prophecy\Argument;

class TwitsControllerSpec extends SpecCase
{
    function let($creator)
    {
        $creator->beADoubleOf('OfficeTwit\Services\Creators\TwitCreator');
        $this->beConstructedWith($creator);
    }

    function it_is_initializable()
    {
        $this->shouldHaveType('Controllers\TwitsController');
    }

    function it_creates_a_twit()
    {
        $this->create();
    }
}

And the extended SpecCase.php

<?php namespace spec;

use PhpSpec\ObjectBehavior;

class SpecCase extends ObjectBehavior {

    public function __construct()
    {
        $this->createApplication();
    }

    /**
     * Creates the application.
     *
     * @return \Symfony\Component\HttpKernel\HttpKernelInterface
     */
    public function createApplication()
    {
        $unitTesting = true;

        $testEnvironment = 'testing';

        return require __DIR__.'/../../../bootstrap/start.php';
    }
}

I also needed to autoload the spec namespace in composer.json

"psr-4": {
    "spec\\": "app/tests/spec",
    "Controllers\\": "app/controllers",
    "OfficeTwit\\": "app/officetwit"
}

Now I can use the aliases in my controller

<?php namespace Controllers;

use OfficeTwit\Services\Creators\TwitCreator;
use Controllers\BaseController;
use Auth; //alias Illuminate\Support\Facades\Auth
use Redirect; //alias Illuminate\Support\Facades\Redirect

class TwitsController extends BaseController {

    protected $creator;

    public function __construct(TwitCreator $creator)
    {
        $this->creator = $creator;
    }

    public function index()
    {
        return View::make('twits.index');
    }

    public function create()
    {
        if(Auth::check())
        {
            $username = Auth::user()->username;

            $input = Input::only('twit');

            if($this->creator->make($input))
                return Redirect::to($username . '/twits');

            return Redirect::back()->withInput()->withErrors($this->creator->getErrors());    
        }

        return Redirect::to('login');

    }

Without being able to use the facades I need to dig through source code to find where they really point to

use Auth; // Illuminate\Auth\Guard
use Redirect; //alias Illuminate\Routing\Redirector (I think)

And even if we use the full path, the functions aren't static so

use Illuminate\Auth\Guard as Auth;

Auth::check();

won't work.

So from my perspective the main thing needed to get this working well in Laravel is a module or config option that somehow automatically generates specs extending a base spec that creates the Laravel application.

Is anyone already working on that or thinking about working on it?

Thanks

@isimmons
Copy link
Author

isimmons commented Mar 2, 2014

I should add that codeception has a laravel4 module that does exactly this. http://codeception.com/docs/modules/Laravel4

"uses bootstrap/start.php to launch"

Though it also adds a bunch of helper methods the main thing is being able to use Laravel's built in mocking for facades and also being able to test classes that may or may not be using facades within them such as Auth, Redirect, Input, etc. These are all facades in Laravel and have their own shouldReceive() method for mocking them.

Might be helpful to see how codeception does it.

@jonphipps
Copy link

Taylor Otwell has a post on his blog that you might find useful: Response: Don't use facades. In that post he also points to a Facade class reference in the Laravel docs.

@stof
Copy link
Member

stof commented Mar 3, 2014

Phpspec is opiniated. It tries to push you to clear object communication. Code using static facade is not OO code. It is procedural code wrapped in a class. This is the reason why you will find it really hard to specify it with PhpSpec. It was an explicit choice while designing the architecture of PhpSpec 2.

@isimmons
Copy link
Author

isimmons commented Mar 3, 2014

@jonphipps Thank you for that class reference link. I had read the post but maybe to hastily :-)

@stoff I can understand that. While I still think a laravel4 module would really spread usage of phpspec into the laravel community more, I can see that I need to re-think how I use phpspec (what things I should and should not be trying to test with it) and how I code if I'm going to use it.

@stof
Copy link
Member

stof commented Mar 3, 2014

Note that I'm @stof. @stoff is someone else.

And a Laravel module hacking against the goals of PhpSpec is a bad idea IMO, even if this could increase the userbase. Such module would harm PhpSpec rather than serving it IMO by breaking its philosophy. People writing procedural code should simply use a testing tool supporting procedural code (with a global state).
And as explained in the blog post linked by @jonphipps above, Laravel does not force you to use procedural code. Code written using encapsulated objects relying on dependency injection will be testable with PhpSpec without requiring any special module.

@cordoval
Copy link
Contributor

cordoval commented Mar 3, 2014

@stof are you saying that the use of such static methods in Laravel often lead people into writing procedural code? Good insight.

@stof
Copy link
Member

stof commented Mar 3, 2014

@cordoval such static method calls are the procedural call. A static call is not OO code (there is no object in it, only a class and a function).

Well, to be exact, using static method calls is not always a sign of procedural code (it is here though). It can also be functional code. But clearly not OO code.

@cordoval
Copy link
Contributor

cordoval commented Mar 3, 2014

i see, nice, so as an artisan I can use both procedural and OO, but understand their difference and their usage of tools and opinions. Thanks for the great explanation @stof

@isimmons
Copy link
Author

isimmons commented Mar 4, 2014

Sorry @stof didn't realize I put a extra 'f' in there. I guess stating that none of the methods behind these facades are actually static methods is irrelevant in this situation since they are being used as if they were static right? I mean the fact that the facade only gives us the ability to call them as if they were static methods. Calling them that way instead of passing them into the constructor is where the problem lies. Just making sure I understand.

So I can choose to use facades as a convenience, just not with classes I want to test with PHPSpec

@stof
Copy link
Member

stof commented Mar 4, 2014

@isimmons What they are doing internally does not change how your code looks like. We don't care about the internal implementation of external code (we are mocking the collaborators anyway so we don't run the internal implementation). We care about the way you communicate with external code.

and the facades are static, even if they don't implementat the actual logic themselves

@MarcelloDuarte
Copy link
Member

@stof, actually façades are accessed statically, but they use the magic staticCall to create a specific object instance (resource, you want to use for doing something). Prophecy, our default mocking framework, does not support mocking the initial static call for the façade entry point. So, @isimmons yeah, go ahead and create a façade to a mocking framework, such as Mockery, to allow for mocking your own façades. Mocking Laravel's façade, or any third party code is not recommended as you loose track of what actually work, adapters are recommended instead.

@stof
Copy link
Member

stof commented Mar 4, 2014

@MarcelloDuarte As far as the app code is running, it is a static call, not an object communication. The way the static call is implemented does not matter at all. It would be the same than talking about the dependencies of your collaborators.

As said previously, the right way to go here is to use DI instead of using the static facades, which is also supported by Laravel (see the blog post linked above)

@MarcelloDuarte
Copy link
Member

@stof I see what you mean. It's that we are composing objects around messages, but invoking methods on objects like we do in structural programming.

Just exchanging ideas with @taylorotwell on twitter, let me add that he has a video on taylorotwell.com explaining how to use Dependency Injection with Laravel. Using "façades" is as good (or bad) as passing a service locator to a class. It's a design choice that comes with a price.

@MarcelloDuarte
Copy link
Member

@isimmons Laravel's author also points out that he only uses façades for Http/Routing. Every step down from there he uses proper DI. If you choose to follow that, phpspec will be a good tool for you

@stof
Copy link
Member

stof commented Mar 4, 2014

@MarcelloDuarte it is worse than passing a service locator actually: if you use DI to access the locator itself, you are still doing object communication (and you will be able to spec it, even if it can be painful due to the setup of the service locator stub returning other mocks). When using a function call (static methods are mostly equivalent to functions) for service location, you are not injecting stuff anymore; You are getting them from some global state.
And such function calls cannot be mocked.

Btw, Mockery allows mocking functions only when

  • the function is in the global scope,
  • the calling code is namespaced,
  • the function is accessed relying on the function fallback (no usage of the fully qualified name \json_encode),
  • there is no function with the same name in the namespace.

Static methods don't satisfy the preconditions of being a PHP function. It is hacking into the PHP function resolution mechanism. And this has lots of drawbacks:

  • you cannot run the code anymore as an integration test in the same PHP process (as the mocked function is now defined in the namespace and used). This would require running the integration test first (which is weird in term of feedback), or to isolate the process used for the unit tests and integration tests
  • you cannot have the real function being called by any other code leaving in the same namespace in this process

So unless your test runner uses process isolation for each test (which is not the case of PhpSpec and slow as hell in PhpUnit), you should better avoid any attempt to mock procedural code

@MarcelloDuarte
Copy link
Member

To make matters worse, looking closer into Illuminate base Façade class, I noticed that it keeps the instance once it's created, like a registry, so should the object created this way rely on any internal state, it will have its state shared across multiple calls to the façade, throwing testability out of the window.

@isimmons
Copy link
Author

isimmons commented Mar 7, 2014

@MarcelloDuarte Yeah actually even using the Http/routing facades wouldn't work because they don't exists unless there is a running instance of the application as is done with Laravel's TestCase and Codeception's L4 module. And automatic dependency resolution doesn't work because the Ioc is part of the Laravel application. A lot of the classes those facades point to also rely on the automatic dependency injection provided by the Ioc.

So as far as I can tell at this point I could use Phpspec for testing my own classes as long as they don't rely on automatic dependency injection or facades.

I had been under the impression that I could replace phpunit with phpspec but I don't think it can be a complete replacement. I think phpunit and phpspec together will work well with testing Laravel apps but as far as I can tell at this point there are just certain things in Laravel that are going to require phpunit such as testing controllers.

@ciaranmcnulty
Copy link
Member

@isimmons If you're doing automated acceptance testing, and your controllers are thin enough, you might not feel like you need to test them at the unit / specBDD level

@JeffreyWay
Copy link

I think his confusion is that he can do a form of functional testing for his controllers with PHPUnit, but he's having trouble doing the same with PHPSpec - because it wasn't built for that style of testing.

So if you're trying to do something like - "When I hit this route, then I should mock that repository method, and then the view should receive some key" - well, that's not what PHPSpec is for.

Use PHPSpec to unit test your domain logic, and then Behat (or Codeception, if you prefer) for your outside-in tests (including controller stuff).

I'd imagine this can be hard for some people who primarily build CRUD-style apps. In these cases, your thought might lean toward "Then what the heck am I supposed to unit test?" This will change, though, as you work on larger apps that have more business requirements.

@isimmons
Copy link
Author

Yep, basically wanted to replace phpunit for these type tests. http://code.tutsplus.com/tutorials/testing-laravel-controllers--net-31456

Plus for other things like say I want to unit test a UserValidator class. If that class in any way depends on the Laravel IoC for automatic DI or the Validator facade it isn't going to work because in that case it won't even pass this basic test

function it_is_initializable()
{
    $this->shouldHaveType('UserValidator');
}

@JeffreyWay
Copy link

Yeah - different style of testing in that article you linked to.

PHPSpec nudges (or forces) you to mock everything. Facades make this very difficult, which is why it's good to not use them outside of your HTTP layer. Instead, inject their underlying classes through your constructor. We can now do that in Laravel quite easily. See Taylor's video on it.

So, the fact that PHPSpec is squawking is a good thing. It's encouraging better code/design habits.

If you want to test a UserValidator, just make sure that you inject any dependencies. That way, you can easily mock them, when you test the class.

@stof
Copy link
Member

stof commented Mar 11, 2014

@isimmons If there is some dependencies in the constructor, you need to add the call to $this->beConstructedWith() to pass the dependencies. It is logical that it is not initializable if you don't pass the arguments. See the PhpSpec doc for this.

And the link you gave is about doing functional tests, which is indeed out of the scope of PhpSpec.

@ericjeker
Copy link

I know this conversation is pretty old but here's a sample code I used to test a form validator.

<?php

namespace spec\Core\Validators;

use Illuminate\Support\MessageBag;
use Illuminate\Validation\Factory;
use Illuminate\Validation\Validator;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class FormValidatorSpec extends ObjectBehavior
{
    function let(Factory $factory)
    {
        $this->beConstructedWith($factory);
    }

    function it_is_initializable()
    {
        $this->shouldHaveType('Core\Validators\FormValidator');
    }

    function it_passes_data_and_rules_to_validator(Factory $factory, Validator $validator, MessageBag $messageBag) {
        $validator->passes()->willReturn(true);
        $factory->make(Argument::any(), Argument::any())->willReturn($validator);
        $this->passes()->shouldReturn(true);

        $data = array('field' => 'data');
        $factory->make($data, Argument::any())->willReturn($validator);
        $this->with($data)->passes()->shouldReturn(true);

        $validator->passes()->willReturn(false);
        $validator->messages()->willReturn($messageBag);
        $this->shouldThrow('Core\Exceptions\FormValidationException')->during('passes');
    }
}

And that's pretty much all I am able to test (and should be testing) using PHPSpec because as soon as I want to test the "real" behavior of the, let's say, UserFormValidator, then I won't be unit testing anymore but I will be testing with collaborators (Validator, Factory, MessageBag, etc...) so those are integration tests.

If I understand well the philosophy, I expect the Validator and Factory to be unit tested on their own and so I should only ensure that the data and rules set in my FormValidator class are correctly passed to the Laravel's validator object because, as a matter of fact, that the only behavior of my FormValidator: to pass data and rules to Illuminate/Validation/Validator and to intercept and transform the answer of that class.

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

8 participants