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

[2.x] Use correct feature flag in password reset tests #881

Merged
merged 2 commits into from
Sep 13, 2021
Merged

[2.x] Use correct feature flag in password reset tests #881

merged 2 commits into from
Sep 13, 2021

Conversation

joelbutcher
Copy link
Contributor

What?

In the fortify routes file, all the routes being tested in both PHPUnit and Pest variants of PasswordResetTest are only exposed using the following condition (see here):

// Password Reset...
if (Features::enabled(Features::resetPasswords())) {
    // ...
}

If the reset-passwords feature flag is disabled, this causes the published test files for PasswordResetTest to fail, regardless of the preferred testing framework used.

This PR updates both PasswordResetTest variants to use the correct feature flag, Features::resetPasswords() instead of Features::updatePasswords() which is currently being used incorrectly.

Why?

I installed a fresh Laravel project with Jetstream using the 2.x-dev branch on packagist to keep one of my packages up-to-date with recent merges / changes. Upon publishing the pest tests and disabling password resets, my CI pipeline broke.

Replicate the problem

You can replicate this on 2.x-dev yourself locally. First, select your stack and publish all files (including tests). Then in config/fortify.php comment out this line:

'features' => [
    // Features::resetPasswords(),
],

Then run your test suite and see the PasswordResetTest's fail.

@taylorotwell taylorotwell merged commit 7baef37 into laravel:2.x Sep 13, 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