Skip to content

Added a test logger#1229

Closed
gmponos wants to merge 2 commits into
Seldaek:masterfrom
gmponos:add_test_logger
Closed

Added a test logger#1229
gmponos wants to merge 2 commits into
Seldaek:masterfrom
gmponos:add_test_logger

Conversation

@gmponos

@gmponos gmponos commented Nov 17, 2018

Copy link
Copy Markdown
Contributor

Hello,

I have created a TestLogger. I hope that you feel that this fits inside your package. I know that Monolog has a TestHandler but I believe this is better for the following reasons:

  • Simpler initialization of Logger inside tests. At the moment in order to perform my tests I need to initialize a Logger and then a TestHandler.. More properties that I have to maintain inside my code... etc etc.
  • Faster tests since it does minimal work. It skips all the extra work that it is made by Logger class.
  • Inside the TestHandler you have the getRecords function. Even if I retrieve all the records when I need to do simple asserts I can not.. Check this:
$this->assertSame(['level' => 'warning', 'message' => 'that', ['context' => []], $records[0]];

The record contains a lot of extra information that in my case they are not needed.

Even in your tests you had to do unset. So I don't need to have all these information like formatted etc etc.

All I need to test is that the API of the LoggerInterface class is called correctly.

@Seldaek

Seldaek commented Nov 19, 2018

Copy link
Copy Markdown
Owner

Could you move this inside a Monolog\Test\ namespace (but still in src/)? If you also wanna help out by taking care of #1197 at the same time that'd be even better :)

@Seldaek

Seldaek commented Nov 19, 2018

Copy link
Copy Markdown
Owner

On the other hand, I kinda wonder if this doesn't belong more in the psr/log package as it doesn't have anything monolog specific really

@Seldaek Seldaek added this to the 2.0 milestone Nov 19, 2018
@gmponos

gmponos commented Nov 20, 2018

Copy link
Copy Markdown
Contributor Author

I had the same dilemma... if I should commit it here or on the PSR. Checking PSR now again I just saw this namespace Psr\Log\Test so yes I believe it could fit in there. check the PR php-fig/log#57

Thank you.

@Seldaek

Seldaek commented Nov 20, 2018

Copy link
Copy Markdown
Owner

Ok gonna close this here then.

@Seldaek Seldaek closed this Nov 20, 2018
@ghost ghost mentioned this pull request Nov 26, 2018
@ghost ghost mentioned this pull request Dec 6, 2018
@gmponos

gmponos commented Nov 10, 2021

Copy link
Copy Markdown
Contributor Author

Hi @Seldaek do you think that this could be added in here now that the TestLogger is removed from psr/log ?

@stof

stof commented Nov 10, 2021

Copy link
Copy Markdown
Contributor

@gmponos as there is still nothing specific to Monolog in that TestLogger (the monolog-specific way is to use the TestHandler of Monolog), you can maintain that in its own package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants