Skip to content

Conversation

adamwathan
Copy link
Contributor

Use a precomputed hash of the word "secret" instead of using bcrypt directly. Since bcrypt is intentionally slow, it can really slow down test suites in large applications that use factories to generate models in many tests.

Here's a reference point from a user who saw their test suite get 5x faster thanks to this change 👀

#3456 (comment)

@GrahamCampbell
Copy link
Member

This is not particularly pretty. There is an easier way to do this to yield the same speedup, without having to hard code a computed hash. Give me a sec...

@GrahamCampbell
Copy link
Member

What do you make of #3897 please?

@adamwathan
Copy link
Contributor Author

@GrahamCampbell Yeah I like it, I think I prefer this to expanded conditional:

$factory->define(App\User::class, function ($faker) {
    static $password = null;

    return [
        'username' => $faker->userName,
        'email' => $faker->safeEmail,
        'password' => $password ?: $password = bcrypt('secret'),
        'remember_token' => str_random(10),
    ];
});

@GrahamCampbell
Copy link
Member

Yeh, that looks prettier. Feel free to update your PR if you're happy with that, and I'll close mine. :)

@adamwathan
Copy link
Contributor Author

Cool done 👍 Thanks for your input!

@adamwathan
Copy link
Contributor Author

Seems like just static $password; would work too, since it assumes null by default. Prettier still?

$factory->define(App\User::class, function ($faker) {
    static $password;

    return [
        'username' => $faker->userName,
        'email' => $faker->safeEmail,
        'password' => $password ?: $password = bcrypt('secret'),
        'remember_token' => str_random(10),
    ];
});

@taylorotwell taylorotwell merged commit a2c075b into laravel:develop Aug 21, 2016
@GrahamCampbell
Copy link
Member

Seems like just static $password; would work too, since it assumes null by default. Prettier still?

Yeh, but only on PHP, not on HHVM. I guess we don't care about HHVM anymore though, so :trollface:.

@driesvints
Copy link
Member

❤️❤️❤️

return [
'name' => $faker->name,
'email' => $faker->safeEmail,
'password' => bcrypt(str_random(10)),
'password' => $password ?: $password = bcrypt('secret'),
Copy link

@yoyosan yoyosan Aug 26, 2016

Choose a reason for hiding this comment

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

Wouldn't using the Hash::make('secret') method from the Hash facade be better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Successfully merging this pull request may close these issues.

9 participants