Testy w Laravel / php

Witam serdecznie,
Uczę się pisania testów w Laravelu. Jako że nie wiem jak powinny wyglądać "profesjonalnie" - to chciałem dopytać czy to, co robię ma sens :)

1. Test 1 - test sprawdzający czy strona działa poprawnie

class HomeTest extends TestCase
{
    public function testHomePageWorkCorrectly()
    {
        //$this->withoutExceptionHandling();
        $response = $this->get('/');

        $response->assertStatus(200);
    }
}

2. Test 2 - test logowania z dobrym i błędnym hasłem:

class LoginTest extends TestCase
{
    public function testUserCanLoginWithCorrectPassword()
    {
        //$this->withoutExceptionHandling();
        $response = $this->post(route('cms.check_admin_login'), [
            'email' => '[email protected]',
            'password' => 'passw'
        ]);
        $response->assertRedirect('/psCMS/');
        $this->assertTrue(Auth::check());
    }

    public function testUserCannotLoginWithInCorrectPassword()
    {
        //$this->withoutExceptionHandling();
        $response = $this->post(route('cms.check_admin_login'), [
            'email' => '[email protected]',
            'password' => 'xxxxx'
        ]);
        $response->assertRedirect('psCMS/login?error=1');
        $this->assertFalse(Auth::check());
    }
}

3. Test 3 - test modułu reklam. Dodawanie nowej reklamy, sprawdzanie czy istnieje, pobieranie listy reklam, sprawdzanie czy walidacja pól działa, kasowanie :

class AdTest extends TestCase
{
    use \Illuminate\Foundation\Testing\WithFaker;

    public function testCreateAd()
    {
        $user = User::find(1);
        $this->be($user);

        $ad = factory(Ad::class)->create();
        $this->post(route('ads.edit') . '/' . $ad->id, $ad->toArray());
        $this->assertDatabaseHas('ads', ['id' => $ad->id]);
    }

    public function testAdExist()
    {
        $user = User::find(1);
        $this->be($user);

        $ad = factory(Ad::class)->create();
        $this->post(route('ads.edit') . '/' . $ad->id, $ad->toArray());
        $this->assertDatabaseHas('ads', ['id' => $ad->id]);
    }


    public function testGetAdsList()
    {
        $user = User::find(1);
        $this->be($user);

        $ads = factory(Ad::class)->create();
        $response = $this->get(route('ads.index'));
        $response->assertSee($ads->title);
    }

    public function testCreateAdMustBeCompleted()
    {
        $user = User::find(1);
        $this->be($user);

        $response = $this->actingAs($user)->post(route('ads.store'), [
            'title' => null,
            'provincial_id' => null,
            'content' => null,
        ]);

        $response->assertSessionHasErrors(['title', 'provincial_id', 'content']);
    }

    public function testCreateAdFromForm()
    {
        $user = User::find(1);
        $this->be($user);
        $faker = Faker\Factory::create('pl_PL');
        $title = $faker->sentence($nbWords = 6, $variableNbWords = true);
        $response = $this->actingAs($user)->post(route('ads.store'), [
            'title' => $title,
            'content' => $faker->text($maxNbChars = 200),
            'provincial_id' => $faker->numberBetween(1, 8),
        ]);

        $this->assertDatabaseHas('ads', ['title' => $title]);
    }


    public function testEditAdFromForm()
    {
        $this->withExceptionHandling();
        $user = User::find(1);
        $this->be($user);
        $faker = Faker\Factory::create('pl_PL');

        $ad = factory(Ad::class)->create();
        $id = $ad->id;

        $title = $faker->sentence($nbWords = 6, $variableNbWords = true);
        $response = $this->actingAs($user)->post(route('ads.update') . '/' . $id, [
            'title' => $title,
            'content' => $faker->text($maxNbChars = 200),
            'provincial_id' => $faker->numberBetween(1, 8),
            'id' => $id
        ]);

        //dd($response->getContent());

        $this->assertDatabaseHas('ads', ['title' => $title, 'id' => $id]);
    }


    public function testDeleteAd()
    {
        //$this->withExceptionHandling();
        //$this->withoutExceptionHandling();
        $user = User::find(1);
        $this->be($user);

        $ad = factory(Ad::class)->create();
        $id = $ad->id;

        $faker = Faker\Factory::create('pl_PL');
        $title = $faker->sentence($nbWords = 6, $variableNbWords = true);
        $response = $this->actingAs($user)->post(route('ads.destroy'), [
            'title' => $title,
            'content' => $faker->text($maxNbChars = 200),
            'provincial_id' => $faker->numberBetween(1, 8),
            //'id'=> $id,
            'id' => [$id]
        ]);

        $this->assertDatabaseMissing('ads', ['id' => $ad->id]);
    }
}

Czy takie testy są poprawne? Czy raczej powinny wyglądać inaczej?:)

w test 1 warto by sprawdzić coś jeszcze poza tym czy response jest ze statusem 200 np jeśli zwracasz coś na stronę to czy te dane się zgadzają.
w test 2 jakieś setup danych do bazy z poziomu testu aby później nie było problemów że testy nie przechodzą bo coś w bazie się zmieniło , chyba że robisz to z poziomu fixtur globalnie do wszystkich testów bo tak też można
w test 3 pierwsze co rzuciło mi się w oczy to może nie związane z samym testem ale do edycji encji używasz metody post a powinien to być put albo patch w zależności od tego czy wysyłasz całą encje czy tylko jej część tak samo do usuwania metoda delete a nie post. 2 pierwsze testy z klasy AdTest są identyczne i robią coś dla mnie nie zrozumiałego mianowicie nazwa pierwszego testu wskazuje na to że sprawdzasz czy reklama się utworzy a wysyłasz na route ads.edit z parametrem id jakieś array poczym sprawdzasz czy ten id jest w bazie bardziej wygląda mi to na edycje , przy edycji nie ma sensu sprawdzania w bazie czy dany id istnieje bo jeżeli by to tam nie było response powinno zwrócić status 404. Ogólnie to ciężko jest mi się konstruktywnie wypowiedzieć na temat testów z 3 klasy ponieważ nie rozumiem logiki którą one testują. Jeszcze jedna kwestia w mojej opinii lepiej mieć w teście statyczne dane niż generować je dynamicznie (faker), ułatwia to debugowanie

mjhallll napisał(a):

w test 1 warto by sprawdzić coś jeszcze poza tym czy response jest ze statusem 200 np jeśli zwracasz coś na stronę to czy te dane się zgadzają.

Test 1 wygląda na zwykły smoke test, czyli wygląda ok. Smoke test ma być szybki do wykonania i ma sprawdzać tylko, czy strona działa (czy zwraca odpowiedni kod statusu). Sprawdzanie, czy dane się zgadzają, itp., to już trochę inne testy.