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

RFC: Namespacing functions #55

Closed
nunomaduro opened this issue May 31, 2020 · 4 comments
Closed

RFC: Namespacing functions #55

nunomaduro opened this issue May 31, 2020 · 4 comments

Comments

@nunomaduro
Copy link
Member

nunomaduro commented May 31, 2020

In the current state of Pest, we could say that PHPUnit static functions like assertions are being injected in the global namespace, and functions specific to the test case are being served thought the $this variable. Here is an example:

<?php

beforeEach(fn () => $this->withoutMiddleware()); // `$this` being used

test('example test', function () {
    assertTrue(true); // `assertTrue` function being used
});

it('has welcome page', function () {
    $this->get('/')->assertSee('Laravel'); // `$this` being used
});

The current approach has a few problems: there is no autocompletion on the $this, and for some developers gets complicated to understand what is available globally and what's available on the $this.

Proposal: Serve namespaced functions for assertions/methods specific to the test case.

✓ No more autocompletion issues
✓ Makes things a little bit more consistent as everything gets available using just simple functions

PHPUnit static functions like assertTrue or assertArrayHas will be still available globally, but assertions/methods specific to the test case would require a use statement. Here is how it could look like:

<?php

use function Pest\Laravel\{get, withoutMiddleware};

beforeEach(fn () => withoutMiddleware());

test('example test', function () {
    assertTrue(true);
});

it('has welcome page', function () {
    get('/')->assertSee('Laravel');
});

The $this is still available for people that want to copy/paste the current PHPUnit test. So no breaking changes are being proposed here.

I am still not sure about this, as it requires one or more extra lines per test. So let me know.

@hivokas
Copy link

hivokas commented May 31, 2020

Won't something like that solve the autocompletion issue?

/** @var $this TestCase */

@owenvoke
Copy link
Member

I think this a good idea. 👍 It would solve the issue for people who dislike the global functions, and also resolve any issues for functions that may be provided by 2 different plugins.

@alexmartinfr
Copy link
Collaborator

alexmartinfr commented Jun 1, 2020

The consistency and ease of use gains trumps the visual burden of the additional line, I would go for it 👍

Plus, real-world tests will also have use statements, and in the end one more line isn't adding too much distraction:

<?php

use App\User;
use Livewire\Livewire;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Hash;
use function Pest\Laravel\{get, withoutMiddleware};

test('can view login page', function () {
    get(route('login'))
        ->assertSuccessful()
        ->assertSeeLivewire('auth.login');
})->group('auth');

test('is redirected if already logged in', function () {
    $user = factory(User::class)->create();

    actingAs($user)
        ->get(route('login'))
        ->assertRedirect(route('home'));
})->group('auth');

[…]

VS

<?php


use App\User;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Hash;
use Livewire\Livewire;
use Livewire\Livewire;

test('can view login page', function () {
    $this->get(route('login'))
         ->assertSuccessful()
         ->assertSeeLivewire('auth.login');
})->group('auth');

test('is redirected if already logged in', function () {
    $user = factory(User::class)->create();

    $this->actingAs($user);

    $this->get(route('login'))
         ->assertRedirect(route('home'));
})->group('auth');

[…]

To me the first example is easier to read, because everything of importance is aligned on a vertical ligne. This allows quick scanning of the important code (this is an information design good practice).

@nunomaduro
Copy link
Member Author

In progress for PHP 0.2.

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

No branches or pull requests

4 participants