thephpleague / factory-muffin

Enables the rapid creation of objects for testing
https://factory-muffin.thephpleague.com/
MIT License
531 stars 72 forks source link

Refactor, deleting the factory muffin facade #334

Closed GrahamCampbell closed 9 years ago

GrahamCampbell commented 9 years ago

yolo

scottrobertson commented 9 years ago

I am not a fan of removing the Facade entirely. It fits some people's needs well. I am in favour of encouraging the use of instances by default though.

GrahamCampbell commented 9 years ago

The facade is a bad idea, and doesn't really make sense. It means that people struggle to customise things because they're in the mind set that they must use a facade, and they must do everything with one object.

GrahamCampbell commented 9 years ago

Without the facade, our definition files are way, way more flexible: https://github.com/thephpleague/factory-muffin/blob/master/tests/factories/main.php. When you call the loadFactories method from an instance of FactoryMuffin, that same instance will be available as the $fm variable. This is a massive improvement as far as I can see. I think removing the facade is definitely the best way to go, and will make sure people think in the correct mind set.

scottrobertson commented 9 years ago

We use separate files for each type of Factory. That means we must instantiate an instance every single file?

scottrobertson commented 9 years ago

It feels really odd having a file that uses a variable that is not set in that file.

GrahamCampbell commented 9 years ago

Laravel is using it in 5.0 for the routes file, for laravel users, it will feel quite normal.

GrahamCampbell commented 9 years ago

We use separate files for each type of Factory. That means we must instantiate an instance every single file?

No, not at all.

scottrobertson commented 9 years ago

for laravel users, it will feel quite normal.

That justifies nothing :P

GrahamCampbell commented 9 years ago

The variable is set like this: https://github.com/thephpleague/factory-muffin/blob/a6f9549e1d1287e5306eb8122723b6e00db4a44b/src/FactoryMuffin.php#L636. And it is documented, and all our documentation uses that variable. I can't see a better way of doing this?

The only other way I can think of would be to use traits.

scottrobertson commented 9 years ago

Just feels really odd that's all. We won't be upgrading for quite some time so it's all fine.

GrahamCampbell commented 9 years ago

We need to decide if we are encouraging people to use out definition loader to require plain php files, or if people should be using traits to define things.

Using Our Loader

# tests/factories/all.php

use League\FactoryMuffin\Faker\Facade as Faker;

$fm->define('Message', array(
    'user_id'      => 'factory|User',
    'subject'      => Faker::sentence(),
    'message'      => Faker::text(),
    'phone_number' => Faker::randomNumber(8),
    'created'      => Faker::date('Ymd h:s'),
    'slug'         => 'Message::makeSlug',
), function ($object, $saved) {
    // we're taking advantage of the callback functionality here
    $object->message .= '!';
});

$fm->define('User', array(
    'username' => Faker::firstNameMale(),
    'email'    => Faker::email(),
    'avatar'   => Faker::imageUrl(400, 600),
    'greeting' => 'RandomGreeting::get',
    'four'     => function() {
        return 2 + 2;
    },
));
# tests/TestUserModel.php

use League\FactoryMuffin\Faker\Facade as Faker;

class TestUserModel extends PHPUnit_Framework_TestCase
{
    protected static $fm;

    public static function setupBeforeClass()
    {
        // create a new factory muffin instance
        static::$fm = new FactoryMuffin();

        // note that method chaining is supported if
        // you want to configure a few extra things
        static::$fm->setSaveMethod('save')->setDeleteMethod('delete');

        // load your factory definitions
        static::$fm->loadFactories(__DIR__.'/factories');

        // you can optionally set the faker locale
        Faker::setLocale('en_EN');
    }

    public function testSampleFactory()
    {
        $message = static::$fm->create('Message');
        $this->assertInstanceOf('Message', $message);
        $this->assertInstanceOf('User', $message->user);
    }

    public static function tearDownAfterClass()
    {
        static::$fm->deleteSaved();
    }
}

Using Traits Loader

# tests/factories/All.php

use League\FactoryMuffin\Faker\Facade as Faker;

Trait All {

    public function defineAll() {
        static::$fm->define('Message', array(
            'user_id'      => 'factory|User',
            'subject'      => Faker::sentence(),
            'message'      => Faker::text(),
            'phone_number' => Faker::randomNumber(8),
            'created'      => Faker::date('Ymd h:s'),
            'slug'         => 'Message::makeSlug',
        ), function ($object, $saved) {
            // we're taking advantage of the callback functionality here
            $object->message .= '!';
        });

        static::$fm->define('User', array(
            'username' => Faker::firstNameMale(),
            'email'    => Faker::email(),
            'avatar'   => Faker::imageUrl(400, 600),
            'greeting' => 'RandomGreeting::get',
            'four'     => function() {
                return 2 + 2;
            },
        ));
    }
}
# tests/TestUserModel.php

use League\FactoryMuffin\Faker\Facade as Faker;

class TestUserModel extends PHPUnit_Framework_TestCase
{
    use All;

    protected static $fm;

    public static function setupBeforeClass()
    {
        // create a new factory muffin instance
        static::$fm = new FactoryMuffin();

        // note that method chaining is supported if
        // you want to configure a few extra things
        static::$fm->setSaveMethod('save')->setDeleteMethod('delete');

        // load your factory definitions
        $this->defineAll();

        // you can optionally set the faker locale
        Faker::setLocale('en_EN');
    }

    public function testSampleFactory()
    {
        $message = static::$fm->create('Message');
        $this->assertInstanceOf('Message', $message);
        $this->assertInstanceOf('User', $message->user);
    }

    public static function tearDownAfterClass()
    {
        static::$fm->deleteSaved();
    }
}