-
-
Notifications
You must be signed in to change notification settings - Fork 77
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
feat: behat support #378
base: 1.x
Are you sure you want to change the base?
feat: behat support #378
Conversation
I'd like this to work seamlessly with dama if possible! |
do you have any problems with dama? |
I don't have the behat tests working yet. I just wanted to note this for when I do (dama has a behat extension) |
I have been thinking a bit about this since we started the discussion in #235, and I am not sure that adding the Behat support in this repo here is the best way to go. I'll try to give reasons by looking at package dependencies.
TL;DR IMO Behat support should live in a dedicated repo/package ( |
Additional remarks after looking at what can be seen over at https://github.com/dmaicher/doctrine-test-bundle/tree/master/src/DAMA/DoctrineTestBundle/Behat, because this was mentioned above. Their code needs to be registered as a Behat extension and will setup listeners to do the setup/teardown work in the database for every test. This may really slow down tests a lot if not all tests (scenarios/examples) need it. By having a Behat context that takes care of this, users are free to work with different Behat test suites and register the context only for those suites where they care. If it even were a Behat "step definition", I could start the relevant scenarios with |
Thanks for the great feedback on this @mpdude! Am I correct the same dependency argument could be made for mongo support? My concern with another package (at this point) is maintenance. I've been burned by this in the past (creating a php package, symfony bundle for this package, laravel provider for this package, etc). I find it's easier to maintain and keep all compatible when they are in a single "mono-repo". In the future, I have no problem creating sub-tree splits (like Symfony core or Flysystem does) to create separate packages. For instance, in the future, we could potentially split mongo/behat into different readonly packages but keep maintenance here. Regarding the behat support itself: Should we have a behat extension + context(s)? If you want to have it available for all scenarios, use the extension. If not, use the contexts on a scenario-by-scenario basis? |
- Zenstruck\Foundry\Tests\Behat\TestContext | ||
- Zenstruck\Foundry\Test\Behat\FactoriesContext | ||
extensions: | ||
FriendsOfBehat\SymfonyExtension: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need some help figuring out why this doesn't seem to be working. When I run vendor/bin/behat
I get the following error:
Can not find a matching value for an argument
$container
of the methodZenstruck\Foundry\Test\Behat\FactoriesContext::__construct()
.
I've confirmed the kernel isn't being booted but not sure why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the problem is to do with this: src/Listener/KernelOrchestrator.php:tearDown in symfony extension...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its loaded in the service extension here:
\FriendsOfBehat\SymfonyExtension\ServiceContainer\SymfonyExtension::loadKernelRebooter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably try to restructure the new foundry behat contexts in this PR as a listener (like KernelOrchestrator) and perhaps try to use the priority (int) on the subscribed callbacks (e.g. 16 or something) to trigger database/foundry registry reset. Ive found it possible to induce memory leaks quite easily or race conditions in a few integrations ive attempted previously.
Yes – just as an example, nothing prevents users from installing Foundry with You could even argue about the Doctrine dependency. In fact I am even using Foundry in parts with a legacy ORM system with lots of Since Foundry describes itself as making "creating fixtures data fun again [...] with Symfony and Doctrine", that's fine. "Everything is a trade-off", they say?
I feel you – have been there as well. Especially when issues start to pop up all over the place and especially in the wrong place. It's not always easy to decide for the uninitiated where an issue or PR should go. I have been maintaining projects/repos where you need to install three or four packages to gather everything you need, and some of those consist of only one or two classes. Basically a system where you have a "core" and then some "import" and "export" adapters around it, with different packages for the
Yes – and note every single one of them comes with their own But how do you keep those subtree splits updated, and how/where will you tag releases when the different parts evolve at a different pace?
Honestly I don't know. I have been using Behat for quite some time now, but never had to write an extension for it. My guess would be:
Note that contexts cannot be used on a per-scenario basis (you probably mean the step definition?). |
Yes, I regret tightly coupling to the ORM (even though it's not required). Maybe we can make some improvements here in 2.0. I'd really like
I think the system Symfony/Flysystem uses is open source so I can investigate this. I'm sort of thinking the following but let me know what you think:
(I don't know if all of the above is possible) |
@kbond Poking around your code... think I got things working, except one part: When/how does the database schema get set up for tests? |
It should be created when the database is reset (initially), then reset (dropped/created) before each scenario (with the current hook-based setup). For the how, I created the |
what Symfony does is to simply tag all components each time a new version is realeased, even if the component did not had any changes. See for example https://github.com/symfony/yaml/releases, notice all releases with |
I believe we no longer make bug-fix tags if there are no changes. |
* Get POC running * Fix after schema @annotation
Thanks for the PR @mpdude, the root of my issue was not passing the env vars... USE_ORM=1 USE_FOUNDRY_BUNDLE=1 DATABASE_URL=mysql://... vendor/bin/behat So how do you think we should proceed with this PR? Do my ideas above make sense? |
Is there are movement on this? I got it wokring with behat, but recent changes have broken things so im looking for something stable going forward for my behat test suite... |
@roboticflamingo, does the plan I laid out in my above comment make sense to you? I have no experience with behat so I'm in need of some direction here. |
@kbond lol I am also in the same boat re behat 3 - there doesnt seem to be much about the internals re documentation. Based on what I know about behat, I would say architecturally yes (with the potential addition of a subscriber - see my comment in code on KernelOrchestrator), however, I think having tried creating something similar the biggest problem is synchronising the kernel reboot, clearing the foundry registry and the database reset to happen together (My first guess is I think your error is a symptom of callback order). BTW ive found foundry to be excellent. Its the best thing out there by far for fixture management. The traits dont work in all situations, but I tore the bits out to integrate it - so was no problem tbh. |
use Foundry for creating test data. Foundry needs to be configured, | ||
started and stopped accordingly. The database has to be reset before | ||
scenarios are run. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PS might be worth adding a test for "example tested" syntax.
https://behat.org/en/latest/user_guide/writing_scenarios.html#scenario-outlines
Associated events for subscriber: ExampleTested::BEFORE, ExampleTested::AFTER
@@ -0,0 +1,15 @@ | |||
Feature: Behat support for Foundry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also probably add an example of using backgrounds to implement the fixtures. I use the back ground to set up core fixtures (that are used in all tests).
https://behat.org/en/latest/user_guide/writing_scenarios.html#backgrounds
} | ||
|
||
/** | ||
* @BeforeScenario |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These annotations cause race conditions because you cannot guarantee when they are run... hence why I think subscriber is the way to go.
In Behat, is a subscriber a different concept than an extension? |
Yes they are different - I think everzet intended them to be a replacement for the annotations - they are Symfony event subscribers - you can load them in the extension using:
|
I am just rewriting mine with the updated code from this repo, that will work with background, examples and scenarios. If I can get it working I will report back here, probably be thursday night though before I get it done or report any issues. Update: It is actually working, but I have to test it with DAMA support! |
https://symfony.com/doc/current/event_dispatcher.html#creating-an-event-subscriber Here are some example events: Note you must register it in the extension (so you might have FoundryExtension and FoundryListener) - KernelOrchestrator in fos symfony extension for behat is a good example actually :): https://github.com/FriendsOfBehat/SymfonyExtension/blob/master/src/Listener/KernelOrchestrator.php |
OK I think I have a working subscriber recipe for this repo... :) |
Fixes #235.