Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] allow tests to utilise the null logger #38785

Merged
merged 1 commit into from
Sep 13, 2021
Merged

[8.x] allow tests to utilise the null logger #38785

merged 1 commit into from
Sep 13, 2021

Conversation

timacdonald
Copy link
Member

@timacdonald timacdonald commented Sep 13, 2021

Note: maybe I'm way off here and don't know how to use the "null" logger - so maybe PEBKEC?

Currently, the "null" logger does not ever get utillised. It instead creates and writes to the emergency logger.

The issue being that when you set this in your env...

# file: .env.testing

LOG_CHANNEL=null

# or...

LOG_CHANNEL='null'

# or

LOG_CHANNEL="null"

It is interpreted as null not "null". There is possibly a way to tell dot env to read this value as a string and I've tried to obvious ones, but didn't have any luck. I'm sure someone will jump in and tell me how, but I reckon a lot of people will have this set and it is triggering the emergency logger instead.

Steps to reproduce the issue...

composer create-project laravel/laravel example-repo
cd example-repo
php artisan key:generate
echo "LOG_CHANNEL=null" > .env.testing
echo "<?php

namespace Tests\Feature;

use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Log\LogManager;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Route;
use Psr\Log\LoggerInterface;
use Tests\TestCase;

class ExampleTest extends TestCase
{
    /**
     * A basic test example.
     *
     * @return void
     */
    public function test_example()
    {
        Route::get('/test', function () {
            Log::info('test');
        });

        \$response = \$this->get('/test');

        \$response->assertStatus(200);
    }
}" > tests/Feature/ExampleTest.php
./vendor/bin/phpunit
cat storage/logs/laravel.log

Hopefully the test is okay. I kinda felt like it would be good to have it all co-located rather than splitting out to atomic tests 🤷‍♂️

I didn't feel comfortable making this impact anything other than unit tests. Maybe this should also happen in production...but on the other hand if this is null in prod you probably really do want it to write to the emergency logger as a safe default.

@taylorotwell taylorotwell merged commit c42c356 into laravel:8.x Sep 13, 2021
victorvilella pushed a commit to cdsistemas/framework that referenced this pull request Oct 12, 2021
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.

2 participants