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

Sail and parallel testing #180

Closed
knobel-dk opened this issue Jan 29, 2024 · 10 comments · Fixed by #179
Closed

Sail and parallel testing #180

knobel-dk opened this issue Jan 29, 2024 · 10 comments · Fixed by #179
Labels

Comments

@knobel-dk
Copy link

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
Copy link
Member

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

laravel new bug-report --github="--public"

@knobel-dk
Copy link
Author

OK. I pushed this to the master branch:

  1. The initial commit after your command: knobel-dk/browser-kit-paratest-bug@41a4fdf
  2. Installed the two packages: knobel-dk/browser-kit-paratest-bug@e5df496
  3. The two tests that shows the difference running with/without Browserkit: knobel-dk/browser-kit-paratest-bug@abff56e

@driesvints
Copy link
Member

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
Copy link
Author

@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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

driesvints commented Feb 1, 2024

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 laravel/sail#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.

@driesvints driesvints reopened this Feb 1, 2024
@driesvints driesvints transferred this issue from laravel/browser-kit-testing Feb 1, 2024
@driesvints driesvints changed the title Browser Kit and Paratest Sail and parallel testing Feb 1, 2024
@knobel-dk
Copy link
Author

@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
Copy link
Member

driesvints commented Feb 1, 2024

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
Copy link
Member

Hi there. We've started working on a fix btw: #179

@driesvints driesvints transferred this issue from laravel/sail Feb 9, 2024
@driesvints driesvints linked a pull request Feb 9, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants