-
Notifications
You must be signed in to change notification settings - Fork 22
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
Use database transactions #12
Conversation
@kielabokkie I will have a look at it tonight 😄Thanks Wouter for the PR 🙂 |
@@ -53,7 +56,7 @@ public static function find(string $mail) | |||
|
|||
if (is_string($dependency) && class_exists($dependency)) { | |||
if (isset($eloquentFactory[$dependency])) { | |||
$args[] = factory($dependency)->states($factoryStates)->make(); | |||
$args[] = factory($dependency)->states($factoryStates)->create(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how are the factories persisted to the db if we are not using DB::commit()
after this? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The factories don't actually have to be persisted in the DB for the view to work. As long as they exist when return new $mailable(...$args);
is called the view is fine. They are never committed so keeps the database clean of data that was created just for the purpose of rendering the email view.
|
||
$this->withFactories(__DIR__ . '/database/factories'); | ||
|
||
$this->loadMigrationsFrom([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need a dummy table? Just curious 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I switched it back from make
to create
for the factory creation the table needs to exist for the test
|
||
return EloquentFactory::construct($faker, $factories_path); | ||
}); | ||
$app['config']->set('app.debug', env('APP_DEBUG', true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, any specific reason for setting this config? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When there is an error somewhere in the code the tests that fail dump the generated error page (because of the assertSee
). Without this setting it is just the standard error page, with this setting enabled you can check the dumped html for the actual error so just helps debugging.
@@ -6,4 +6,5 @@ | |||
|
|||
class Test extends Model | |||
{ | |||
public $timestamps = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this? I think I might be missing something 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the Test model is persisted to the database for the tests I either had to add $table->timestamps();
to the migration or just remove the timestamps for the model. I decided to go for the last one as the timestamps are irrelevant for the test anyway 😉
@kielabokkie also, can you rename the |
@introwit I've fixed the folder names and tried to answer your questions. Let me know if anything is still unclear 😃 |
@kielabokkie Tagged a new release 😄 Thank you again for the great contribution Wouter ❤️ |
@introwit I'm back with another PR! 😁
I had some trouble with factories that use the afterMakingState function to create related models for hasOne relationships like below:
Figured out that using
create
instead ofmake
for the factory would solve it. Problem was that the test would fail because the database can't be accessed. I utilised testbench' ability to setup an in-memory sqlite database. The other problem with creating the factories is that they are persisted to the database which is ok on dev and testing environments but not so much on staging and production, I've used transactions to make sure nothing ever gets stored in the db.I also found a cleaner way to load the factories using testbench so also tidied that up.