jrdavidson's avatar

Refactoring Test Methods

I'm trying to clean up some old code from a project and trying to figure out some ideas on renaming the test functions in a better way so that I nor others have to scratch their head to try to understand what I'm attempting to do. Also any suggestions on if there are areas in the test that I could modify.

/** @test */
    public function a_non_title_match_can_set_a_winner()
    {
        $wrestlerA = factory(Wrestler::class)->create();
        $wrestlerB = factory(Wrestler::class)->create();
        $match->addWrestlers([
            0 => [$wrestlerA],
            1 => [$wrestlerB]
        ]);

        $this->match->setWinner($wrestlerA, 'pinfall');

        $this->assertEquals($wrestlerA->id, $this->match->winner_id);
        $this->assertEquals($wrestlerB->id, $this->match->loser_id);
    }

    /** @test */
    public function a_title_match_champion_can_be_set_as_winner()
    {
        $wrestlerA = factory(Wrestler::class)->create();
        $title = factory(Title::class)->create();
        $title->setChampion($wrestlerA, Carbon::parse('2018-03-03'));

        $event = factory(Event::class)->create(['date' => '2018-03-05']);
        $match = factory(Match::class)->create(['event_id' => $event->id]);
        $wrestlerB = factory(Wrestler::class)->create();
        $match->addWrestlers([
            0 => [$wrestlerA],
            1 => [$wrestlerB]
        ]);
        $match->addTitle($title);

        $match->setWinner($wrestlerA, 'dq');

        $this->assertEquals($wrestlerA->id, $match->winner_id);
        $this->assertEquals($wrestlerB->id, $match->loser_id);
        $this->assertEquals($wrestlerA, $title->currentChampion);
    }

    /** @test */
    public function a_title_changes_hands_when_a_non_champion_wins_a_title_match()
    {
        $wrestlerA = factory(Wrestler::class)->create(['name' => 'Wrestler A']);
        $title = factory(Title::class)->create();
        $title->setChampion($wrestlerA, Carbon::parse('2018-03-03'));

        $event = factory(Event::class)->create(['date' => Carbon::parse('2018-03-05')]);
        $match = factory(Match::class)->create(['event_id' => $event->id]);
        $wrestlerB = factory(Wrestler::class)->create(['name' => 'Wrestler B']);
        $match->addWrestlers([
            0 => [$wrestlerA],
            1 => [$wrestlerB]
        ]);
        $match->addTitle($title);

        $match->setWinner($wrestlerB, 'pinfall');

        $title->refresh();
        $this->assertEquals($wrestlerB->id, $match->winner_id);
        $this->assertEquals($wrestlerA->id, $match->loser_id);
        $this->assertEquals($wrestlerB->id, $title->currentChampion->id);
    }
0 likes
6 replies
bobbybouwmann's avatar

Well, just describe what the tests do! You already have a pretty describable method name, but you might want to describe what kind of winner it is in the method name. For example the pinfall or dq string?

I have no clue about the rest of the domain, so helping out is a bit harder without context!

1 like
BrandonSurowiec's avatar

You could leverage your factories to generate some of the other models.

Your match could automatically generate the event as needed.

// In your Match Factory

[
   'event_id' => factory(Event::class)->lazy(),
]

You could also leverage states to encapsulate some of the factory generation.

$title = factory(Title::class)->states('withChampion')->create();

Then you could even leverage $title->currentChampion as one of the fighters.

1 like
jrdavidson's avatar

@BrandonSurowiec for the following code I thought about it but I also have a pivot table between a wrestler and a title that is contained in a TitleChampion model. I also have a factory for this model. I am not sure if using a factory for getting a title champion is the best so I"m wondering if you agree and would suggest keeping with your approach and just doing the state withChampion on the Title factory.

$factory->define(App\Models\Champion::class, function (Faker\Generator $faker) {
    return [
        'wrestler_id' => factory(App\Models\Wrestler::class)->lazy(),
        'title_id' => factory(App\Models\Title::class)->lazy(),
        'won_on' => Carbon::now(),
    ];
});
BrandonSurowiec's avatar

For the most bang for your buck test these methods through the endpoint where you are using them, and not directly like a unit test.

That way:

A. You'll know your implementation works

B. If you change your implementation your test shouldn't have to change.

C. You'll stop duplicating the code that is in your controller.

Here's a rewrite of the first method on how I would want to write it.

function test_a_non_title_match_can_set_a_winner()
{
    // By default, let's assume your match factory auto create two wrestlers.
    $match = factory(Match::class)->create();

    $winner = $match->wrestlers->first();
    $loser = $match->wrestlers->last();

    $this->post("match/{$match->id}", [
        'winner_id' => $winner->id,
        'won_by' => 'pinfall',
    ]);

    $match->refresh();
    $this->assertEquals('pinfall', $match->won_by);
    $this->assertTrue($winner->is($this->match->winner));
    $this->assertTrue($loser->is($this->match->loser));
}

I would probably leverage states() with the Match class and have that generate everything that you need so you can follow a similar format as the above..

jrdavidson's avatar

@BrandonSurowiec I know about states but I"m not sure how I could attach wrestlers for a match beacuse its a belongsToMany relationship for both.

$factory->define(App\Models\Match::class, function (Faker\Generator $faker) {
    return [
        'event_id' => factory(App\Models\Event::class)->lazy(),
        'match_number' => $faker->numberBetween(6, 10),
        'match_type_id' => factory(App\Models\MatchType::class)->lazy(),
        'stipulation_id' => factory(App\Models\Stipulation::class)->lazy(),
        'preview' => $faker->paragraphs(3, true)
    ];
});
BrandonSurowiec's avatar

You can simply make a new class like /database/MatchFactory.php. I've done this lots of times.

<?php

class MatchFactory
{
    function create($attributes = [])
    {
        $match = factory(Match::class)->create($attributes);

        // Whatever you need to do to generate wrestlers..
        $match->wrestler()->saveMany([
            factory(Wrestler::class)->create(),
            factory(Wrestler::class)->create(),
        ]);

        return $match;
    }
}

Then you just use that.

function test_a_non_title_match_can_set_a_winner()
{
    // By default, let's assume your match factory auto create two wrestlers.
    $match = (new MatchFactory)->create();

    $winner = $match->wrestlers->first();
    $loser = $match->wrestlers->last();

The cool thing about this, is you can do whatever you want. You can add assertion helper methods, or config options to the constructor on how many wrestlers to create, etc.

Please or to participate in this conversation.