Migrate BrowserKit tests to HTTP tests

[fabiorodriguesroque]

Tests to migrate

• AdminTest
• ArticleTest
• AuthTest
• BlockUsersTest
• CanonicalUrlTest
• ForumTest
• HomeTest
• ModeratorTest
• NavigationTest
• NotificationsTest
• ProfileTest
• ReplyTest
• SettingsTest
• SubscriptionsTest

[fabiorodriguesroque]

@joedixon could you make a quick review on the AdminTest migration before I start migrate all other test cases?

Basically I've updated

1. assertRedirectedTo() to ->assertRedirectToRoute($name, $parameters = []);
2. seeInDatabase to assertDatabaseHas
3. notSeeInDatabase to assertDatabaseMissing
4. see to assertSee
5. dontSee to assertDontSee

About the first point would you prefer use assertRedirectTo($uri) instead of assertRedirectToRoute?

Thanks.

[joedixon]

Great work @fabiorodriguesroque, thank you!

All looks good to me. I don't think I'm particularly bothered if you use assertRedirectTo vs assertRedirectToRoute. In an ideal world, we'd probably use the route name since we have them configured in the routes file, but in reality, I don't think they have ever changed and it's actually nice, in my opinion, if the tests catch when they do.

[driesvints]

@fabiorodriguesroque thanks a bunch for this! I agree with @joedixon and think assertRedirectTo is probably preferred.

[fabiorodriguesroque]

Yes! I got your point, the AdminTest was already updated with the AssertRedirect() and I'll follow this for the next ones.

[fabiorodriguesroque]

@driesvints @joedixon

I've updated almost all tests with exception of AuthTest, ForumTest and SettingsTest, where I think I need to use Laravel Dusk.

for example

test('users can register', function () {
    Notification::fake();

    session(['githubData' => ['id' => 123, 'username' => 'johndoe']]);

    $this->visit('/register')
        ->type('John Doe', 'name')
        ->type('john.doe@example.com', 'email')
        ->type('johndoe', 'username')
        ->check('rules')
        ->check('terms')
        ->press('Register')
        ->seePageIs('/user/johndoe')
        ->see('John Doe');

    assertLoggedIn();

    $this->assertSessionMissing('githubData');

    Notification::assertSentTo(Auth::user(), VerifyEmail::class);
});

Should I add the laravel/dusk dependency?? otherwise we can't migrate all use cases.

[driesvints]

Wonder if we can migrate this one to just a HTTP test. I think we only need to migrate this part, yes?

        ->seePageIs('/user/johndoe')
        ->see('John Doe');

Skip the last part and migrate seePageIs to assertRedirectedTo or something?

[joedixon]

Yes, I think an HTTP test is fine here too 👍

[fabiorodriguesroque]

Wonder if we can migrate this one to just a HTTP test. I think we only need to migrate this part, yes?

        ->seePageIs('/user/johndoe')
        ->see('John Doe');

Skip the last part and migrate seePageIs to assertRedirectedTo or something?

yes, the only thing here is the functions like type() check() but I can use actingAs() and send a normal post or put request.

I've migrated the SettingsTest with actingAs($user) and updated the submitForms() to put() or post().

[joedixon]

yes, the only thing here is the functions like type() check() but I can use actingAs() and send a normal post or put request.

I've migrated the SettingsTest with actingAs($user) and updated the submitForms() to put() or post().

This is exactly what I had in mind, thank you!

[driesvints]

Hey @fabiorodriguesroque. Anything we can do to help get this PR over the finish line? We really appreciate this one! :)

[fabiorodriguesroque]

Hey @fabiorodriguesroque. Anything we can do to help get this PR over the finish line? We really appreciate this one! :)

Yes @driesvints first of all sorry for not giving any news for a long time, but I'v been a little busy!
I'm with some difficulties with the ForumTest in two tests!

I will commit and open a review!
Thanks

[fabiorodriguesroque]

resolved ✅

[joedixon]

This is totally awesome @fabiorodriguesroque, thank you 🙌

[driesvints]

Amazing work @fabiorodriguesroque. Really appreciate this. Thanks!

[driesvints]

Seems for some reason we did miss the SubscriptionsTest here. Not sure what went wrong and how tests could pass 🤔

[fabiorodriguesroque]

Seems for some reason we did miss the SubscriptionsTest here. Not sure what went wrong and how tests could pass 🤔

Weird! I was to take a look on this, but noticed you already migrate that one and deleted the BrowserKitTestCase, my bad. Thanks.

[driesvints]

Yeah just pushed it. No worries. The test suite should have caught this so no idea why it didn't.