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

Fix Pest generated tests #726

Merged

Conversation

nicodevs
Copy link
Contributor

@nicodevs nicodevs commented Jan 2, 2025

On a clean installation, the generated Pest tests fail because assertActionUsesFormRequest is an undefined method.

Steps to replicate

Installation:

# Create Laravel project
composer create-project laravel/laravel test
cd test
# Install Blueprint
composer require -W --dev laravel-shift/blueprint
# Install Pest
composer remove phpunit/phpunit
composer require pestphp/pest --dev --with-all-dependencies
composer require pestphp/pest-plugin-faker --dev
composer require pestphp/pest-plugin-laravel --dev
./vendor/bin/pest --init
# Install Laravel Test Assertions
composer require --dev jasonmccreary/laravel-test-assertions

Initialize Blueprint:

php artisan blueprint:new --config

Comment out PHPUnit and uncomment Pest:

// 'test' => \Blueprint\Generators\PhpUnitTestGenerator::class,
'test' => \Blueprint\Generators\PestTestGenerator::class,

Create draft:

models:
  Post:
    title: string
controllers:
  Post:
    resource: web

Generate:

php artisan blueprint:build

Run tests:

./vendor/bin/pest

Tests fail with the following error:

   FAILED  Tests\Feature\Http\Controllers\PostControllerTest > store uses form request validation         ReflectionException
  Call to undefined method Tests\TestCase::assertActionUsesFormRequest()

  at tests/Feature/Http/Controllers/PostControllerTest.php:33
     29▕ });
     30▕
     31▕
     32▕ test('store uses form request validation')
  ➜  33▕     ->assertActionUsesFormRequest(
     34▕         \App\Http\Controllers\PostController::class,
     35▕         'store',
     36▕         \App\Http\Requests\PostStoreRequest::class
     37▕     );

  1   tests/Feature/Http/Controllers/PostControllerTest.php:33

Reason

This happens for two reasons:

  1. The PestTestGenerator is not calling addTrait to add AdditionalAssertions.
  2. The PestTestGenerator uses the trait HandlesTraits, but Pest tests manage traits differently: pest()->use().

Solution

Tests relying on assertActionUsesFormRequest should include the following:

  • This line at the top of the file:
use JMac\Testing\Traits\AdditionalAssertions;
  • This line before the tests:
pest()->use(AdditionalAssertions::class);

This PR achieves that outcome by adding this logic when processing a ValidateStatement:

$this->addImport($controller, 'JMac\\Testing\\Traits\\AdditionalAssertions');
$this->addTrait($controller, 'AdditionalAssertions');

The PestTestGenerator now uses its own addTrait method instead of relying on HandlesTraits. Given that this method is specific to PestTestGenerator and not shared across other classes, introducing it locally rather than creating a new concern seemed appropriate.

@nicodevs nicodevs marked this pull request as ready for review January 2, 2025 22:51
@jasonmccreary
Copy link
Collaborator

Thanks. Is this ready, or a WIP?

@nicodevs
Copy link
Contributor Author

nicodevs commented Jan 3, 2025

You're welcome @jasonmccreary! Yes, this is ready.

@jasonmccreary jasonmccreary merged commit d49ef4e into laravel-shift:master Jan 3, 2025
25 checks passed
@jasonmccreary
Copy link
Collaborator

Merged this before taking that close of a look. Two things I'm curious about:

  • Could you have used HandlesTraits, but overwritten the build method?
  • Do you think it makes more sense when they are generating Pest tests to add this trait globally instead of every test file?

@nicodevs
Copy link
Contributor Author

nicodevs commented Jan 3, 2025

Thanks!

  1. That makes sense. Here's the PR: Make PestTestGenerator use HandlesTraits trait #727
  2. The trait can be added to the Pest.php file:
pest()->extend(Tests\TestCase::class)->use(AdditionalAssertions::class);

...or to the base test case, but I was unsure if it would be acceptable to modify files outside those generated by Blueprint. Let me know if you'd like me to take that approach!

@jasonmccreary
Copy link
Collaborator

Thanks.

Fair point. Blueprint does modify the routes files. So doing so for Pest, especially if it streamlines code, should be ok. Assuming this is still how Pest does it - common config in a base Pest file?

@nicodevs
Copy link
Contributor Author

nicodevs commented Jan 3, 2025

Sounds good! Tomorrow, I'll give it a go. In the meantime, here's a small PR to fix a test related to this feature: #729

@nicodevs nicodevs deleted the fix-pest-failing-generated-test branch January 5, 2025 12:44
@nicodevs
Copy link
Contributor Author

nicodevs commented Jan 6, 2025

@jasonmccreary, here's the PR to Import AdditionalAssertions to base test case when generating Pest tests: #733

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