chilio/laravel-dusk-ci

Should we remove the 10s sleep after initializing laravel ?

ice-blaze opened this issue · 7 comments

On this line

I would see an environment variable instead of an hard code 10s.

We could move this to ENV, but it is needed to ensure system chromedriver is started before the one shipped with laravel.

I see two solutions here:

  • Having an environment variable, and if not defined, use the 10 seconds as default.
  • Wait until the chrome driver is really started. With a while loop waiting on the driver.

Which one do you prefer? (I would say the cleanness is the second one)

For sure second one is much more cleaner...

@ice-blaze second option is now embedded in dev.
Can you verify, if it is working, the way you expected?

Hi, wow thanks I thought I had to put my head into this feature ^^'

Well I still have the sleep 10s https://github.com/chilio/laravel-dusk-ci/blob/dev/commands/configure-laravel.sh#L14 in the configure-laravel

I tried without the sleeep 10s and it worked but it also work on the stable release so I guess my PC is too fast.

Hi,
sure I forgot to remove the sleep....
And you are true, if it works without sleep on your machines, they are fast enough.
This was intended to doublecheck for really slow CI environments, and your suggested solution saves few seconds...

@chilio Sure, thank you for the modifications! Issue closed :)