Add integration tests against a Symfony app #39
Conversation
| <service id="fos_http_cache.proxy.log_subscriber" | ||
| class="FOS\HttpCache\EventListener\LogSubscriber" | ||
| abstract="true"> | ||
| <argument type="service" id="logger" /> |
There was a problem hiding this comment.
would this need a on-invalid="true"?
There was a problem hiding this comment.
This is not necessary as the service is defined as abstract. The LoggerPass will then make this service concrete only if the logger service is available.
There was a problem hiding this comment.
okay. the compiler pass plus config allow more than one way how to do things. makes sense, yes.
There was a problem hiding this comment.
Yeah, I created the LoggerPass because I wasn’t sure how to implement the conditional method call (addSubscriber) in service XML. If you know a way, the LoggerPass may not be needed.
There was a problem hiding this comment.
when we can, we do such things in the DependencyInjection class (e.g. with the cmf) but i think in this case we can't know if the logger will be present or not.
i suggest we leave as is and hopefully eventually can simplify.
There was a problem hiding this comment.
Exactly, for services outside the bundle you only know which ones will be available at compiler pass stage.
|
cool, looking good so far. i don't see the actual test here so i guess its not finished? |
|
You’re right. I added ‘[WIP]’ to mark this unfinished. |
|
Okay, added the first functional test. Have a look if you’re happy with the way I set it up. |
| ->addDefaultsIfNotSet() | ||
| ->treatFalseLike(array('enabled' => false)) | ||
| ->treatTrueLike(array('enabled' => true)) | ||
| ->treatNullLike(array('enabled' => true)) |
There was a problem hiding this comment.
is this not the same as using canBeEnabled ?
There was a problem hiding this comment.
It seems it is. I just copied from the other listeners in our config, so let’s change it there, too.
|
this looks really cool! impressive how little setup is needed, i like it. and seems there is quite some bits to cleanup, i like the compiler pass to fail early and with a clear message what is wrong. |
Fix local Varnish IP Rename services.xml to cache_manager.xml Log to file not stdout
Improve doc Remove unused fixture
Simplify config with canBeEnabled() and canBeDisabled()
|
Simplified the config and squashed commits. Please merge if you agree with the setup. We can then open separate PRs to add more functional tests and fix any issues that come up. |
|
looks good yes. thanks! i still see 6 commits - if you want to keep them, merge as is, otherwise push the squashing and then merge :-) |
|
Yep, I’d like to keep those six. 😉 |
Add integration tests against a Symfony app
No description provided.