Skip to content
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

Change the location of screenshots and console folders #393

Closed
vpratfr opened this issue Oct 20, 2017 · 7 comments
Closed

Change the location of screenshots and console folders #393

vpratfr opened this issue Oct 20, 2017 · 7 comments

Comments

@vpratfr
Copy link

vpratfr commented Oct 20, 2017

The tests folder is PSR-4 compliant. Those two folders do not really belong here.

They would rather be located in storage/logs/dusk/...

Moving the folders is currently possible with a custom setUp method in the DuskTestCase class:

protected function setUp()
{
    parent::setUp();

    Browser::$storeScreenshotsAt = storage_path('logs/dusk/screenshots');
    Browser::$storeConsoleLogAt = storage_path('logs/dusk/console');
}

However, the DuskCommand complains that the folders in tests/Browser do not exist when trying to clean them.

I think that for the same reasons some folders/files were taken out of the app folder from L4 to L5, we should keep the tests folder free of non-PSR-4 stuff.

@deleugpn
Copy link
Contributor

I don't think there's any issue with having stubs, fakes, mocks, logs or screenshots inside test folder.

@vpratfr
Copy link
Author

vpratfr commented Oct 20, 2017

Well, having the routes folder inside the app directory in Laravel has never been an issue either. That said, since app became a PSR4 compliant folder, they decided to keep things clean and organised.

Not saying that those folders MUST be moved. Just saying that there is a way to tell the browser to store them somewhere else, but the artisan command does not honour that choice.

At the very least, I would not mind the command not cleaning up my custom folders if it did not simply crash because the default folders are missing (in which case it should just skip the cleanup step).

@staudenmeir
Copy link
Contributor

I added an is_dir() check to purgeScreenshots() and purgeConsoleLogs(): #577

@theomessin
Copy link

Any update on this?

@driesvints
Copy link
Member

driesvints commented Feb 12, 2020

@theomessin you're free to attempt a pr for this.

@derekmd
Copy link
Contributor

derekmd commented May 15, 2020

@driesvints: The recent pull request to allow configurable directories was declined so this should probably be closed.

However, the DuskCommand complains that the folders in tests/Browser do not exist when trying to clean them.

in the root issue post was fixed by staudenmeir 2 years ago. tests/DuskTestCase@setUp() in an app can still customize these paths without needing a config setting.

@driesvints
Copy link
Member

@derekmd thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants