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

Test methods influence each other through session #100

Closed
fschwaiger opened this issue Feb 3, 2017 · 20 comments
Closed

Test methods influence each other through session #100

fschwaiger opened this issue Feb 3, 2017 · 20 comments

Comments

@fschwaiger
Copy link

Test methods should not influence each other. However, the following two methods share the same session and the second will fail, because the user is already logged in.

    public function test_login_redirects_on_first_time()
    {
        $this->browse(function (Browser $browser) {
            $browser->visit('/') // has RedirectIfAuthenticated middleware to /dashboard
                ->type('username', 'johndoe')
                ->type('password', 'secret')
                ->press('@login')
                ->assertPathIs('/dashboard');
        });
    }

    public function test_session_from_previous_method_should_not_exist()
    {
        $this->browse(function (Browser $browser) {
            $browser->visit('/')
                ->assertPathIs('/');  // actually is /dashboard
        });
    }

The issue is fixed if the last line, closeAllButPrimary() is changed to closeAll():

    public function browse(Closure $callback)
    {
        $browsers = $this->createBrowsersFor($callback);

        try {
            $callback(...$browsers->all());
        } catch (Exception $e) {
            $this->captureFailuresFor($browsers);

            throw $e;
        } catch (Throwable $e) {
            $this->captureFailuresFor($browsers);

            throw $e;
        } finally {
-            static::$browsers = $this->closeAllButPrimary($browsers);
+            static::$browsers = $this->closeAll($browsers);
        }
    }
@Sieabah
Copy link

Sieabah commented Feb 4, 2017

You should drop the session each test instead of killing the entire chromedriver process. Plus, why aren't you mocking the session in your tests? Or set the session to Array

@fschwaiger
Copy link
Author

Mocking the session is not possible since the browser is a different process.
Ensuring the test environment is clean should be the job of browse(...), setUp() or tearDown().

@JVMartin
Copy link

JVMartin commented Feb 7, 2017

This is an issue for me as well. The session and the cache persist from test to test, and there's no way to use "array" for either. So all those files need to be deleted between every test.

I'm currently achieving this by setting them both to "file" and then using the following in my DuskTestCase.php:

/**
 * Set up our testing environment
 *
 * @return void
 */
public function setUp()
{
	parent::setUp();
	exec('git clean -fxd ' . storage_path());
}

(This deletes all the session and cache files.)

@georaldc
Copy link

georaldc commented Feb 7, 2017

@Sieabah You can't use array because as already mentioned, the actual tests execute under a different process. Probably the same reason you cannot use in-memory sqlite. One way to circumvent these problems is to use a physical sqlite file (if you want to use sqlite), set both cache and session drivers to use the database and use the DatabaseMigrations trait to continually reset your database before every test. Of course, this means you would need the migration files for both cache and session tables, even if you will be using something else on a different environment like production.

Because of this though (running on a separate process), I just realized that this prevents me from setting up my application environment such as mocking services and disabling middleware since anything that happens in my DuskTestCase classes won't exist during the actual test runs. Any way around this?

@JVMartin
Copy link

JVMartin commented Feb 7, 2017

@georaldc Just wanna make sure you didn't miss that I gave code above to circumvent the problems without using database driver for session/cache.

Also, for way faster testing, you can simply run your migrations and seeds once beforehand, creating a prepared.sqlite file and then use it as a fresh copy of your database on every test so you don't have to keep re-running your migrations and seeds.

This ends up looking like this (in tests/DuskTestCase.php):

/**
 * Set up our testing environment
 *
 * @return void
 */
public function setUp()
{
	parent::setUp();

	// Reset the sqlite testing database
	// http://www.chrisduell.com/blog/development/speeding-up-unit-tests-in-php/
	copy(database_path('prepared.sqlite'), database_path('database.sqlite'));

	// Reset the cache and sessions.
	exec('git clean -fxd ' . storage_path('framework/cache'));
	exec('git clean -fxd ' . storage_path('framework/sessions'));
}

@georaldc
Copy link

georaldc commented Feb 7, 2017

@JVMartin yup, saw that. Was just giving an alternative solution. Running migrations happen very quickly for me so I am not too concerned about it affecting speed (maybe if I have a ton of tests would I look for a different approach, such as your suggestion so thanks for that). I have bigger speed related problems though when using dusk, which can be found in a separate issue I created a few days ago - #98

@JVMartin
Copy link

JVMartin commented Feb 7, 2017

As @georaldc pointed out, the issues with the new architecture are deep, and prevent all manner of integration testing that used to be possible.

For instance:

public function testForgotPasswordFlow()
{
	Mail::fake();

	// Use the forgot password form.
	$this->browse(function(Browser $browser) use ($user) {
		$browser->visit(new HomePage)
			->clickLink('forgot password?')
			->type('@forgot-password-email', '[email protected]')
			->press('Email Me')
			->waitForText(trans('auth.forgot-pass'));
	});

	Mail::assertSent(ResetPasswordLinkEmail::class);
}

The assertSent() will always fail. This kind of testing no longer works because the application itself is now completely decoupled from the testbed.

How are we supposed to do integration tests now?

@SebastianS90
Copy link
Contributor

@JVMartin regarding mails you could configure laravel to submit mails to mailtrap, then use their API to verify that the mail has been sent when executing your Dusk tests. There is this great inboxes/{inbox_id}/messages endpoint that lists you all mails in your inbox.

But I guess the more Laravel way would be to implement what I've sketched in #152. That would also be a lot faster than sending actual mail over the network to mailtrap and pulling the data back into your test case for verification. And you could make it work for anything where you can set the driver in the .env file.

@mtmail
Copy link

mtmail commented Feb 23, 2017

Another work-around is using https://github.com/themsaid/laravel-mail-preview and then fetching the HTTP from there. Of course don't enable it on production systems. Drawbacks: hard to parse the subjects line (it's inside a HTML comment) and you only have access to one (the last) email. It saves the emails to a text file, similar to the Dusk screenshots.

@deleugpn
Copy link
Contributor

I found this topic while searching about Laravel Session and it worried me a little because I was unable to reproduce this problem. This is what I had as my test:

	/**
	 * @test
	 * @return void
	 */
	public function it_should_login() {
		$user = factory(Admin::class)->create();
		$this->browse(function (Browser $browser) use ($user) {
			$browser->visit('/')
				->type('email', $user->email)
				->type('password', $this->password)
				->press('Login')
				->assertSee(__('Dashboard'));
		});
	}
	
	/**
	 * @test
	 * @return void
	 */
	public function it_should_see_email_error_message() {
		$this->browse(function (Browser $browser) {
			$browser->visit('/')
				->press('Login')
				->assertSee(__('The email field is required.'))
				->assertSee(__('The password field is required.'));
		});
	}

So I thought: why am I not facing this issue? The only thing that came to mind was the following:

// Start the database from database.sql
$mysql = 'mysql -D' . getenv('DB_DATABASE') . ' < ' . __DIR__ . '/../database/database.sql';
exec($mysql);

Each test will have a clean database because of these lines that I have on createApplication, which means I was only unable to reproduce this problem because when it_should_see_email_error_message is called, there is no user with id 1 inside the database (because it was created from scratch again). If I just add $user = factory(Admin::class)->create(); to the begin of the test, it is enough to cause it to fail because then the user with id 1 that is considered logged can be found in the database.

@fschwaiger
Copy link
Author

fschwaiger commented Feb 24, 2017

I agree! I just moved sessions to the database and this is a nice workaround. The tests pass immediately.

> php artisan session:table
> php artisan migrate

Edit .env.dusk:

SESSION_DRIVER=database

EDIT: Note this only works if you use DatabaseMigrations.
To improve, you can edit the migration file to only create the sessions table in testingenvironment.

@sileence
Copy link
Contributor

@fschwaiger that seems to be a very clever solution, thank you!

@SebastianS90
Copy link
Contributor

How about Laravel Passport that uses JWT cookies instead of sessions?

@inxilpro
Copy link
Contributor

A quick fix:

Inside your browse() closure, run:

$browser->driver->manage()->deleteAllCookies();

I'm going to submit a PR for this as well.

@inxilpro
Copy link
Contributor

Another option until my PR is approved. Add this to your root DuskTestCase:

public function browse(Closure $callback)
{
    parent::browse($callback);
    static::$browsers->first()->driver->manage()->deleteAllCookies();
}

@ghost
Copy link

ghost commented Aug 23, 2017

For localStorage tokens like those you might set using a JWT solution, you could do the following

    $this->browse(function (Browser $browser) use ($admin) {
        $browser->script("localStorage.clear()");
        $browser->visit('/dash')
            ->on(new DashboardLoginPage)
            ->assertPathIs('/dash/login');
    });

You can execute scripts on the browser using $browser->script() in this case i'm using JS to clear the localStorage before executing a test that requires the user not be signed in.
I was using $this->closeAll() before but this is much much faster.

@Jamesking56
Copy link

@smtlk That localStorage solution doesn't work for me, I get an error:

Facebook\WebDriver\Exception\NoScriptResultException: <unknown>: Failed to read the 'localStorage' property from 'Window': Storage is disabled inside 'data:' URLs.

@driesvints
Copy link
Member

Closing this issue because it's already solved, old or not relevant anymore. Feel free to reply if you're still experiencing this issue.

@ryancwalsh
Copy link

@inxilpro I appreciate the deleteAllCookies() suggestion, but it hasn't worked. https://stackoverflow.com/a/49451157/470749 worked for a while on certain tests, but on tests I'm writing today, according to Dusk screenshots in my tests, my Laravel login session continues even after calling foreach (static::$browsers as $browser) { $browser->driver->manage()->deleteAllCookies(); }. It's so bizarre!

@MoogyG
Copy link

MoogyG commented May 8, 2019

Same things here, I suspect $browser->driver->manage()->deleteAllCookies(); to be asynchronous, it could explain these random results.
The Curl call of facebook driver in /vendor/facebook/webdriver/lib/Remote/HttpCommandExecutor.php is synchronous but maybe Chrome is queuing the task so the next Dusk test could be running before cookie deletion.

I didn't find a universal solution to handle all sessions drivers, you have to make your own.

For me I had to change SESSION_DRIVER from redis to file and use exec('git clean -fxd ' . storage_path('framework/sessions'));

It would be great to have an artisan command to delete all sessions, regardless of session driver.

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

No branches or pull requests