laracasts / Behat-Laravel-Extension

Laravel extension for Behat functional testing.
MIT License
260 stars 73 forks source link

Fix for #78 (also linked to #80): Update to be compatible with Laravel 5.6 #81

Closed philtweir closed 5 years ago

philtweir commented 6 years ago

When used with Behat/Behat@826f32 (now merged into Behat/Behat@master), this allows all relevant references to be serialized by Symfony DI.

Symfony 4.x replaces json_encode with serialize, which throws an exception, instead of failing silently, when it comes across closures. This removes a closure in Behat-Laravel-Extension, specifically by replacing the app object with a reference to it. The Behat commit above removes another, and so the DI serialization can complete successfully.

The recent Behat commit is a soft dependency - both are required for L5.6 to work, but this commit is otherwise independent of it and should not break BC (as it swaps $app for a reference already inserted to DI by Behat-Laravel-Extension).

osman-mohamad commented 6 years ago

why this pull request has not been accepted ? I use Laravel 5.6 . and this pull request fix the error when using this extension with laravel 5.6

roperto commented 6 years ago

why this pull request has not been accepted ? I use Laravel 5.6 . and this pull request fix the error when using this extension with laravel 5.6

+1 here

jcornide commented 6 years ago

why this pull request has not been accepted ? I use Laravel 5.6 . and this pull request fix the error when using this extension with laravel 5.6

Same here! Please merge this :)

xabierlegasa commented 6 years ago

+1 here, please merge! :)

osman-mohamad commented 6 years ago

@xabierlegasa I cloned the repository locally and fixed the bug . and used the local clone in my composer.json file .

dpslwk commented 6 years ago

Readme will want updating to revert #73

philtweir commented 6 years ago

Good point - will double-check it works by simply dropping the explicit symfony/dependency-injection requirement (seems like the most logical approach) before I update.

jcornide commented 5 years ago

Any news on this? we still are not able to use the extension with the new versions of laravel

osman-mohamad commented 5 years ago

@jcornide I forked the repository . cloned it locally . and fixed the problem with this pull request . and then used the local clone for my application . by setting it in the composer.json file. do the same . this repository (package) is not maintained by any one any more .

jcornide commented 5 years ago

@jcornide I forked the repository . cloned it locally . and fixed the problem with this pull request . and then used the local clone for my application . by setting it in the composer.json file. do the same . this repository (package) is not maintained by any one any more .

So you've created your own github laravel extension repo and use that instead of this one?

philtweir commented 5 years ago

Double-checked this still runs off the branch fine on a fresh Laravel project with the pinned symfony dep removed (readme now updated).

@jcornide you can use one of our existing forks if preferred, if you add the repo to the repositories section of the composer.json - obviously, that's not a great long term alternative to using official releases, but it should solve the immediate problem.

@alnutile Would it be possible to merge this please, if there's no issues?

yeroon commented 5 years ago

I can confirm this fix works with Laravel 5.6. Please merge!

philtweir commented 5 years ago

Thank you @alnutile !

jcornide commented 5 years ago

Thanks @alnutile!