jrdavidson's avatar

Testing An Obese Test

Critique but let me know what can I do to refactor this large test.

function user_can_view_a_published_event_listing()
    {
        $venue = factory(Venue::class)->create([
            'name' => 'The Cool Venue',
            'address' => '123 Example Lane',
            'city' => 'Laraville',
            'state' => 'FL',
            'zip' => '12345',
        ]);

        $event = factory(Event::class)->states('published')->create([
            'title' => 'Example Event',
            'date' => Carbon::parse('December 2, 2016 8:00pm'),
            'venue_id' => $venue->id,
            'preview' => 'Just a preview for the event.',
        ]);

        $matchTypeOne = factory(MatchType::class)->create([
            'name' => 'Singles'
        ]);

        $matchTypeTwo = factory(MatchType::class)->create([
            'name' => 'Triple Threat'
        ]);

        $matchTypeThree = factory(MatchType::class)->create([
            'name' => 'Tag Team'
        ]);

        $matchTypeFour = factory(MatchType::class)->create([
            'name' => 'Four Corners'
        ]);

        $stipulationOne = factory(MatchStipulation::class)->create([
            'name' => 'I Quit'
        ]);

        $titleOne = factory(Title::class)->create([
            'name' => 'World Tag Team Titles'
        ]);

        $matchOne = factory(Match::class)->create([
            'event_id' => $event->id,
            'match_number' => 1,
            'type_id' => $matchTypeOne->id,
            'stipulation_id' => $stipulationOne->id
        ]);

        $matchTwo = factory(Match::class)->create([
            'event_id' => $event->id,
            'match_number' => 2,
            'type_id' => $matchTypeTwo->id,
        ]);

        $matchThree = factory(Match::class)->create([
            'event_id' => $event->id,
            'match_number' => 3,
            'type_id' => $matchTypeThree->id,
            'title_id' => $titleOne->id
        ]);

        $matchFour = factory(Match::class)->create([
            'event_id' => $event->id,
            'match_number' => 4,
            'type_id' => $matchTypeFour->id,
        ]);

        $wrestler1 = factory(RosterMember::class)->create([
            'name' => 'Wrestler 1'
        ]);

        $wrestler2 = factory(RosterMember::class)->create([
            'name' => 'Wrestler 2'
        ]);

        $wrestler3 = factory(RosterMember::class)->create([
            'name' => 'Wrestler 3'
        ]);

        $wrestler4 = factory(RosterMember::class)->create([
            'name' => 'Wrestler 4'
        ]);

        $wrestler5 = factory(RosterMember::class)->create([
            'name' => 'Wrestler 5'
        ]);

        $wrestler6 = factory(RosterMember::class)->create([
            'name' => 'Wrestler 6'
        ]);

        $wrestler7 = factory(RosterMember::class)->create([
            'name' => 'Wrestler 7'
        ]);

        $wrestler8 = factory(RosterMember::class)->create([
            'name' => 'Wrestler 8'
        ]);

        $wrestler9 = factory(RosterMember::class)->create([
            'name' => 'Wrestler 9'
        ]);

        $tagteam1 = factory(RosterMember::class)->create([
            'name' => 'Tag Team 1'
        ]);

        $tagteam2 = factory(RosterMember::class)->create([
            'name' => 'Tag Team 2'
        ]);

        $this->assertEquals($event->venue_id, $venue->id);
        $this->assertEquals($matchOne->type_id, $matchTypeOne->id);
        $this->assertEquals($matchTwo->type_id, $matchTypeTwo->id);
        $this->assertEquals($matchThree->type_id, $matchTypeThree->id);
        $this->assertEquals($matchFour->type_id, $matchTypeFour->id);

        $matchOne = $event->matches()->save($matchOne);
        $matchTwo = $event->matches()->save($matchTwo);
        $matchThree = $event->matches()->save($matchThree);
        $matchFour = $event->matches()->save($matchFour);

        $matchOne->competitors()->save($wrestler1);
        $matchOne->competitors()->save($wrestler2);
        $matchTwo->competitors()->save($wrestler3);
        $matchTwo->competitors()->save($wrestler4);
        $matchTwo->competitors()->save($wrestler5);
        $matchThree->competitors()->save($tagteam1);
        $matchThree->competitors()->save($tagteam2);
        $matchFour->competitors()->save($wrestler6);
        $matchFour->competitors()->save($wrestler7);
        $matchFour->competitors()->save($wrestler8);
        $matchFour->competitors()->save($wrestler9);

        $this->visit('/events/'.$event->id);

        $this->see('Example Event');
        $this->see('December 2, 2016');
        $this->see('8:00pm');
        $this->see('The Cool Venue');
        $this->see('123 Example Lane');
        $this->see('Laraville, FL 12345');
        $this->see('Just a preview for the event.');
        $this->see('Opening Match');
        $this->see('Singles Match');
        $this->see('Wrestler 1 vs. Wrestler 2');
        $this->see('Match #2');
        $this->see('Triple Threat Match');
        $this->see('Wrestler 3 vs. Wrestler 4 vs. Wrestler 5');
        $this->see('Match #3');
        $this->see('Tag Team Match');
        $this->see('World Tag Team Titles Match');
        $this->see('Tag Team 1 vs. Tag Team 2');
        $this->see('Main Event');
        $this->see('Four Corners Match');
        $this->see('Wrestler 6 vs. Wrestler 7 vs. Wrestler 8 vs. Wrestler 9');
    }
0 likes
9 replies
jrdavidson's avatar

I'm trying to get some help understanding what I should refactor out of this test.

jrdavidson's avatar

Hoping I can get someone to shed some light here.

ohffs's avatar

(I'm a bit tipsy - it's hogmany here! so forgive me if this is all rubbish)

I think you're maybe trying to 'see' too much in the one test? Maybe think about breaking it down into something like :

  • an_event_can_have_multiple_matches
  • a_match_can_have_many_wrestlers
  • a_wrestler_can_be_removed_from_a_match
  • etc.

etc. Then you can do a small unit test something like :

$match = factory(...);
$match->addWrestlers(...);
$wrestlers = $match->formattedWrestlerList();
$this->assertEquals('Jim vs. Carol vs. Big Daddy', $wrestlers);

Then your main 'see' test can be a lot smaller and less specific as you're just need to test the basics as you know the underlying stuff works.

You've also got a lot of assertions which aren't really testing very much. For instance you do :

$event = factory(Event::class)->states('published')->create([... 'venue_id' => $venue->id, ...]);
...
$this->assertEquals($event->venue_id, $venue->id);

You're pretty much testing eloquent itself there rather than anything of your own logic. If you had something like :

$venue = factory(....);
$venue->addEvent($event1Data);
$venue->addEvent($event2Data);

Then you could maybe make some assertions that the 'addEvent' method was doing what you expected, but doing things like :

$model->property = 2;
$this->assertEquals(2, $model->property);

Doesn't really get you much :-)

1 like
jrdavidson's avatar

@ohffs Well this is what I have for my unit tests of a Match. What changes would you make to it.

<?php

use App\Match;
use App\Extentions\MatchCollection;

class MatchTest extends TestCase
{
    /** @test */
    function can_format_the_first_match_name_of_a_collection_of_matches()
    {
        $matches = new MatchCollection ([
            factory(Match::class)->make(['match_number' => 1]),
            factory(Match::class)->make(['match_number' => 2]),
            factory(Match::class)->make(['match_number' => 3]),
            factory(Match::class)->make(['match_number' => 4])
        ]);

        $this->assertEquals('Opening Match', $matches->getMatchName($matches->first()));
    }

    /** @test */
    function can_format_a_single_match_name()
    {
        $matches = new MatchCollection ([
            factory(Match::class)->make(['match_number' => 1])
        ]);

        $this->assertEquals('Main Event', $matches->getMatchName($matches->first()));
    }

    /** @test */
    function can_format_the_last_match_of_a_collection_of_matches()
    {
        $matches = new MatchCollection ([
            factory(Match::class)->make(['match_number' => 1]),
            factory(Match::class)->make(['match_number' => 2]),
            factory(Match::class)->make(['match_number' => 3]),
            factory(Match::class)->make(['match_number' => 4])
        ]);

        $this->assertEquals('Main Event', $matches->getMatchName($matches->last()));
    }

    /** @test */
    function can_format_a_match_that_is_not_the_first_match_or_last_match()
    {
        $matches = new MatchCollection ([
            factory(Match::class)->make(['match_number' => 1]),
            factory(Match::class)->make(['match_number' => 2]),
            factory(Match::class)->make(['match_number' => 3]),
            factory(Match::class)->make(['match_number' => 4])
        ]);

        $this->assertEquals('Match #2', $matches->getMatchName($matches->get(1)));
        $this->assertEquals('Match #3', $matches->getMatchName($matches->get(2)));
    }
}

jrdavidson's avatar

Anybody else think this is too much and needs changed?

jrdavidson's avatar

@ohffs I was watching the video series from Adam Wathan and he shows all kinds of $this->see() test methods to make sure that when you land on a page you see certain data.

Please or to participate in this conversation.