phergie / phergie-irc-bot-react

IRC bot built on React
BSD 2-Clause "Simplified" License
81 stars 26 forks source link

Bugfix: When setting the logger of the bot, we have to change the log… #31

Closed iwalkalone69 closed 8 years ago

iwalkalone69 commented 8 years ago

…ger of irc connection to take effect.

elazar commented 8 years ago

Can you update this test accordingly, please?

iwalkalone69 commented 8 years ago

Test updated.

elazar commented 8 years ago

Note the comment above this test indicating that it's intended to be a unit test. Your change makes it rely on the behavior of the client dependency so that it becomes an integration test.

To correct this:

$client = $this->getMockClient();
$this->bot->setClient($client);
$this->bot->setLogger($logger);
Phake::verify($client)->setLogger($logger);

This makes it a unit test again because it no longer relies on the behavior of the setLogger() and getLogger() methods of the client dependency, only verifies that setLogger() is called with the correct parameter.

iwalkalone69 commented 8 years ago

Updated test.

elazar commented 8 years ago

@iwalkalone69 Sorry, I did not mean to imply that my example constituted the whole test. You've removed the assignment for $logger, which is still needed.

enebe-nb commented 8 years ago

Probably is this what elazar wants:

    public function testSetLogger()
    {
        $client = $this->getMockClient();
        $logger = $this->getMockLogger();
        $this->bot->setClient($client);
        $this->bot->setLogger($logger);
        $this->assertSame($logger, $this->bot->getLogger());
        Phake::verify($client)->setLogger($logger);
    }
elazar commented 8 years ago

@enebe-nb Not sure @iwalkalone69 is still monitoring this issue. If you'd care to re-submit this PR with appropriate fixes, I'll be happy to merge it. :)

iwalkalone69 commented 8 years ago

Sorry. I had to leave this for a while because I had to focus on another thing. I will take care now.

elazar commented 8 years ago

@iwalkalone69 No worries, looks like @enebe-nb beat you to it. ;)

iwalkalone69 commented 8 years ago

That's right! Thanks @elazar and @enebe-nb !