laracasts / TestDummy

Easy factories for PHP integration testing.
https://laracasts.com/lessons/whats-new-in-testdummy
MIT License
457 stars 80 forks source link

`$faker->optional()` is not getting called foreach transaction #101

Open manuphatak opened 9 years ago

manuphatak commented 9 years ago

$faker->optional() is only getting called ONE time per Seeder. So the results are either ALL filled or ALL empty instead of healthy mix of each.

Problem

Isolated example:

<?php // tests/factories/TimeEntryFactory.php

/** @var Closure $factory */
$factory('App\TimeEntry',
         [
             'user_id'    => 'factory:App\User',
             'comment'    => $faker->optional($weight = 0.5)->sentence()
         ]);

and

<?php // database/seeds/TimeEntryTableSeeder.php

use Illuminate\Database\Seeder;
use Illuminate\Support\Facades\DB;
use Laracasts\TestDummy\Factory as TestDummy;

class TimeEntryTableSeeder extends Seeder
{

    public function run()
    {
        DB::table('time_entry')->delete();

        TestDummy::times(100)->create('App\TimeEntry');
    }
}

You would expect about 50/100 entries to have null "comments", instead it's randomly either 0 or 100.

Solution:

Using for loop instead of times() did not work.

Using a for loop, pulling in Faker, overriding the factory did.

Work around:

<?php // database/seeds/TimeEntryTableSeeder.php

use Illuminate\Database\Seeder;
use Illuminate\Support\Facades\DB;
use Faker\Factory as Faker;
use Laracasts\TestDummy\Factory as TestDummy;

class TimeEntryTableSeeder extends Seeder
{

    protected $faker;

    function __construct(Faker $faker)
    {
        $this->faker = $faker->create();
    }

    public function run()
    {
        DB::table('time_entry')->delete();

        for ($i = 0; $i <= 100; $i++) {
            TestDummy::create(
                'App\TimeEntry',
                ['comment' => $this->faker->optional($weight = 0.5)->sentence()]
            );
        }
    }
}

With this solution, the results are no longer ALL or NOTHING, it looks 50/50.

phroggyy commented 9 years ago

What about using a closure instead? Would that not solve the issue or am I thinking in the wrong track?

phroggyy commented 9 years ago

I can't see why that'd make a difference though, since the create method runs a for loop over times

ht2 commented 8 years ago

Hi @bionikspoon,

Your workaround was very helpful. Thanks for this

Pete