laravel / browser-kit-testing

Provides backwards compatibility for BrowserKit testing in the latest Laravel release.
MIT License
509 stars 75 forks source link

Sail and parallel testing #180

Closed knobel-dk closed 9 months ago

knobel-dk commented 9 months ago

Browser Kit Testing Version

7.1

Laravel Version

10.10

PHP Version

8.3

Database Driver & Version

MySQL 8

Description

I noticed that BrowserKit does not read the correct database with ParaTest. Replicated on a fresh Laravel Sail install today, see below to reproduce or run tests here: https://github.com/knobel-dk/browser-kit-paratest-bug.

Steps To Reproduce

With Sail:

Clone https://github.com/knobel-dk/browser-kit-paratest-bug and run tests will give:

ParaTest v7.3.1 upon PHPUnit 10.5.9 by Sebastian Bergmann and contributors.

Processes:     8
Runtime:       PHP 8.3.2-1+ubuntu22.04.1+deb.sury.org+1
Configuration: /var/www/html/phpunit.xml

F.                                                                  2 / 2 (100%)

Time: 00:00.848, Memory: 12.00 MB

There was 1 failure:

1) Tests\Feature\ExampleTest::test_the_application_returns_a_successful_response
Failed asserting that 'testing' [ASCII](length: 7) contains "testing_test_" [ASCII](length: 13).

/var/www/html/tests/Feature/ExampleTest.php:19

FAILURES!
Tests: 2, Assertions: 2, Failures: 1.

Starting from scratch

curl -s "https://laravel.build/example-app" | bash
cd example-app && ./vendor/bin/sail up
sh vendor/bin/sail composer require laravel/sanctum laravel/browser-kit-testing brianium/paratest

Copy these three files into the repo:

  1. https://github.com/knobel-dk/browser-kit-paratest-bug/blob/main/tests/TestCase.php
  2. https://github.com/knobel-dk/browser-kit-paratest-bug/blob/main/tests/Unit/ExampleTest.php
  3. https://github.com/knobel-dk/browser-kit-paratest-bug/blob/main/tests/Feature/ExampleTest.php

Run this to get the output above:

sh vendor/bin/sail php artisan test -p
driesvints commented 9 months ago

Thanks. Please re-create the repo with the below command and commit all custom changes separately.

laravel new bug-report --github="--public"
knobel-dk commented 9 months ago

OK. I pushed this to the master branch:

  1. The initial commit after your command: https://github.com/knobel-dk/browser-kit-paratest-bug/commit/41a4fdff898afe77abb7b935128ef3aa00b37588
  2. Installed the two packages: https://github.com/knobel-dk/browser-kit-paratest-bug/commit/e5df4964a3d187ba65fb88a80a5f6e25a8a4cb96
  3. The two tests that shows the difference running with/without Browserkit: https://github.com/knobel-dk/browser-kit-paratest-bug/commit/abff56e8b2c37bab73d47304b1e5527c1b3e50ba
driesvints commented 9 months ago

You have your DB_DATABASE set to the hardcoded value of testing here: https://github.com/knobel-dk/browser-kit-paratest-bug/blob/main/phpunit.xml#L24

Remove that and you'll get something similar as the below (because DB_DATABASE is set to laravel for me in my .env):

1) Tests\Unit\ExampleTest::test_that_true_is_true
Failed asserting that 'laravel_test_1' [ASCII](length: 14) contains "testing_test_" [ASCII](length: 13).

/Users/driesvints/Herd/bug-parallel/tests/Unit/ExampleTest.php:16
/Users/driesvints/Herd/bug-parallel/vendor/laravel/framework/src/Illuminate/Foundation/Testing/TestCase.php:177

2) Tests\Feature\ExampleTest::test_the_application_returns_a_successful_response
Failed asserting that 'laravel' [ASCII](length: 7) contains "testing_test_" [ASCII](length: 13).

/Users/driesvints/Herd/bug-parallel/tests/Feature/ExampleTest.php:19
knobel-dk commented 9 months ago

@driesvints Removing the entry in phpunit.xml does not work on the original Sail example, I shared with you.

The Sail is out-of-the-box, so your setup is not realistic.

Even if it was: How about updating docs for either Parallel testing or Browser Kit then, no? It's a default install.

driesvints commented 9 months ago

Removing the entry in phpunit.xml does not work on the original Sail example, I shared with you.

I really doubt this is Sail specific tbh.

The Sail is out-of-the-box, so your setup is not realistic.

What setup is that? I'm just using your reproduction repo.

Even if it was: How about updating docs for either Parallel testing or Browser Kit then, no? It's a default install.

Could you point out which exact docs you mean?

knobel-dk commented 9 months ago

I really doubt this is Sail specific tbh.

Not saying it is Sail-specific. I am saying it is general and that your working example is the odd one out. In my work setup, it does not work either. I reproduced for your convenience with out-of-the-box Sail.

What setup is that? I'm just using your reproduction repo.

Please see my instructions in original post in this issue. It takes four commands in the CLI to reproduce.

Could you point out which exact docs you mean?

You adviced to remove the entry in phpunit.xml to get Laravel\BrowserKitTesting\TestCase to use the right DB name when using ParaTest. If that was the solution (which it is not) the BrowserKit docs should state that this entry must be removed from phpunit.xml unlike Illuminate\Foundation\Testing\TestCase, which works out the database name with ParaTesting differently.

driesvints commented 9 months ago

So I took a deep dive into this and it turns out nothing to do with browserkit testing. Sail sets this testing database env variable in phpunit.xml. That was done since https://github.com/laravel/sail/pull/388. With the following example:

<?php

namespace Tests\Feature;

use App\Models\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Tests\TestCase;

class ExampleTest extends TestCase
{
    use RefreshDatabase;

    /**
     * A basic test example.
     */
    public function test_the_application_returns_a_successful_response(): void
    {
        $databaseName = (new User)->getConnection()->getDatabaseName();
        $this->assertStringContainsString('testing_test_', $databaseName);
    }
}

When I run sail artisan test -p I get:

1) Tests\Feature\ExampleTest::test_the_application_returns_a_successful_response
Illuminate\Database\QueryException: SQLSTATE[HY000] [1044] Access denied for user 'sail'@'%' to database 'testing' (Connection: mysql, SQL: drop database if exists `testing_test_2`)

So this leads me to believe there's a permission issue. I'll bring this up internally.

Moving this issue to the Sail repo.

knobel-dk commented 9 months ago

@driesvints Thanks, I am just noting

  1. It happens in my own setup which is not Sail (which resembles this https://laravel-news.com/laravel-scheduler-queue-docker)
  2. This happens only with Laravel\BrowserKitTesting\TestCase and not the ordinary TestCase

My two cents is that it is related to BrowserKitTesting and not Sail

driesvints commented 9 months ago

Thanks. If I get the above working again, I'll try again with the browser kit testing testcase as well. Note that in the above reproduction example I do not use browserkit testing at all.

driesvints commented 9 months ago

Hi there. We've started working on a fix btw: https://github.com/laravel/browser-kit-testing/pull/179