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

[5.6] Updated to use new release of cron-expression #21637

Merged

Conversation

dragonmantank
Copy link
Contributor

This updates the Laravel Framework to support the new 2.x branch of cron-expression.

I looked through the test set to fix what I could find that relies on the library, and the only test I could not fix was Illuminate\Tests\Console\Scheduling\EventTest::testBuildCommand ... I'm just not exactly sure how all that is managed, so the hash it generates is different than what the test expects. If I missed any other tests, let me know and I'll gladly update this PR. Most of the test changes are because cron-expression has dropped support for the Year place.

This is in response to mtdowling/cron-expression#153 and #19532. More detail on the package name change can be found at http://ctankersley.com/2017/10/12/cron-expression-update/

@Dylan-DPC-zz
Copy link

The Illuminate\Tests\Console\Scheduling\EventTest::testBuildCommand test is failing

@dragonmantank
Copy link
Contributor Author

Right, I mention that above. I don't know exactly what that test is doing in the first place (I don't use Laravel) so I'm not sure what the proper fix is. While I could just change the hash, I don't know that it's supposed to change. Any help would be appreciated with that test.

@taylorotwell
Copy link
Member

Thanks I'll take a look

@dragonmantank
Copy link
Contributor Author

Thanks @taylorotwell !

@themsaid
Copy link
Member

Fixed the test by applying the new mutex after the expression has changed.

@taylorotwell taylorotwell merged commit 07d160a into laravel:master Oct 13, 2017
@taylorotwell
Copy link
Member

Thanks

dragonmantank referenced this pull request in illuminate/console Oct 22, 2017
* Updated to use new release of cron-expression

*    fix test
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.

None yet

4 participants