jrdavidson's avatar

Trying Out Some Refactoring For My Login Test

I figure I'm pretty much done with my login testing and wanting to see if anyone felt like I was missing something or should redo how I"m assigning properties from the setUp function. Please look carefully as I might have made some mistakes.

<?php

use Illuminate\Foundation\Testing\WithoutMiddleware;
use Illuminate\Foundation\Testing\DatabaseMigrations;
use Illuminate\Foundation\Testing\DatabaseTransactions;

class LoginTest extends TestCase
{
    use DatabaseTransactions;

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

        $this->signIn(['email' => 'john@example.com', 'password' => bcrypt($this->password)]);
    }

    /** @test */
    public function a_user_can_successfully_log_in()
    {
        $this->visit(route('login'));
        $this->type($this->user->email, 'email');
        $this->type($this->password, 'password');
        $this->press('Login');
        $this->seePageIs(route('dashboard'));
    }

    /** @test */
    public function a_user_receives_errors_for_wrong_login_credentials()
    {
        $this->visit(route('login'));
        $this->type($this->user->email, 'email');
        $this->type('testpass456', 'password');
        $this->press('Login');
        $this->see('These credentials do not match our records.');
    }

    /** @test */
    public function a_user_is_redirected_to_dashboard_if_logged_in_and_tries_to_access_login_page()
    {
        $this->visit(route('login'));
        $this->seePageIs(route('dashboard'));
    }


}


<?php

class TestCase extends Illuminate\Foundation\Testing\TestCase
{
    /**
     * The base URL to use while testing the application.
     *
     * @var string
     */
    protected $baseUrl = 'http://localhost';

    protected $user;

    /**
     * Creates the application.
     *
     * @return \Illuminate\Foundation\Application
     */
    public function createApplication()
    {
        $app = require __DIR__.'/../bootstrap/app.php';

        $app->make(Illuminate\Contracts\Console\Kernel::class)->bootstrap();

        return $app;
    }

    public function signIn($user = null)
    {
        if (! $user) {
            $user = factory(App\User::class)->create();
        }

        $this->user = $user;

        $this->actingAs($this->user);

        return $this;
    }
}
0 likes
13 replies
MikeHopley's avatar

I would say it looks good.

The only thing I'm a bit confused about is the signIn method. You seem to be using this to log in a user before the test begins, and then in the first two tests you...try to log in the user again. This seems odd, because normally you need to log out first, and I don't understand how the pre-test signInis helping you anyway.

It does work nicely for the third test though.

Here are the questions I ask myself about a test:

  1. Is it reliable?
  2. Is it comprehensive?
  3. Does it operate at the right level?
  4. Is it readable and maintainable?

(1) is the most important. If the production code breaks, then the test should fail.

(2) does the test cover all the ways that this method/class/feature can be used? For example, does it cover failed actions as well as successful ones?

(3) should this be a unit test? Should it be hitting a database? Should I have multiple test levels for this item (e.g. one unit test, one test that hits a database)?

(4) how easy is it to read and maintain this test? Did I need to write a lot of convoluted setup code, e.g. lots of complicated mocks? Does the test depend on my default database seeding, or is it self-contained?

I would say your test satisfies all these considerations.

nfauchelle's avatar

@xtremer360

One issue is your first test and last test contradict each other.

In your setUp you are logging in, and then you first test you go to the login page and try and login.

But in the last test you expect to be redirected.

I suggest renaming the $this->signIn( function to setupUser and make it so it doesn't login (just creates a standard user). Then assigning the $this->email and $this->password to the class; making sure to use it before it's encrypted.

Also in the first test you seem to use $this->password but I can't see where it's assigned currently, so changing the above will solve that.

nfauchelle's avatar
nfauchelle
10 years ago
Best Answer
Level 11

@xtremer360 ie, your test could be changed to this. --

<?php

use Illuminate\Foundation\Testing\WithoutMiddleware;
use Illuminate\Foundation\Testing\DatabaseMigrations;
use Illuminate\Foundation\Testing\DatabaseTransactions;

class LoginTest extends TestCase
{
    use DatabaseTransactions;

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

        $plainPassword = 'test';
        $this->user = factory(App\User::class)->create([
           'email' => 'john@example.com',
           'password' => bcrypt($plainPassword),
        ]);
        $this->user->plainPassword = $plainPassword;
    }

    /** @test */
    public function a_user_can_successfully_log_in()
    {
        $this->visit(route('login'));
        $this->type($this->user->email, 'email');
        $this->type($this->user->plainPassword, 'password');
        $this->press('Login');
        $this->seePageIs(route('dashboard'));
    }

    /** @test */
    public function a_user_receives_errors_for_wrong_login_credentials()
    {
        $this->visit(route('login'));
        $this->type($this->user->email, 'email');
        $this->type('invalid-password', 'password');
        $this->press('Login');
        $this->see('These credentials do not match our records.');
    }

    /** @test */
    public function a_user_is_redirected_to_dashboard_if_logged_in_and_tries_to_access_login_page()
    {
        $this->actingAs($this->user);
        
        $this->visit(route('login'));
        $this->seePageIs(route('dashboard'));
    }


}

and if needed, move out lines 15-20 of the setup to the parent class and call it setupUser

1 like
MikeHopley's avatar

@MikeHopley So you are saying remove it from the first two tests?

Yes, it looks like that would make more sense.

But it's possible I've misunderstood your code or intent. Can you explain why the signIn method is used in the first two tests?

jrdavidson's avatar

@nfauchelle Actually when I run your tests it says integrity constraint because even new test that runs in the LoginTest class will recreate that same user again so not sure what I need to do now.

jrdavidson's avatar

@nfauchelle So what I think is happening is its running the create for the first function and its running then reruns the second function and creates the same user again. I think that's what's happening because the first test isn't erroring but the second and third do.

Please or to participate in this conversation.