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

Update Symfony2 framework module and connector, fix client generation… #2968

Merged
merged 1 commit into from Apr 12, 2016

Conversation

vyarmolenko
Copy link
Contributor

…, update Doctrine EM retrieval. Improve memory usage. Unify service retrieval.

This PR provides solution for #2938, #2954.

This PR proposes the following model of Symfony2 framework module and client:

  • module's $container is a read-only property. It returns active client's service container during test execution or self service container between tests when accessed externally.
  • the module has $persistentServices array which contains services to persist between requests (kernel reboots) in a test.
  • the module has $permanentServices array which contains services to persist between tests.
  • persistence status of any service can be changed at any moment during test execution (public methods persistService(...) / unpersistService(...)).
  • the app kernel is booted during module initialization in order to make framework features available before client creation.
  • a client is created before each test. It is created based on current kernel, currently retrieved persistent services and a configuration option marking client's kernel rebootable between requests or not. Client's kernel is rebooted in its constructor to ensure clean environment for test.
  • client's persistent services are being updated before kernel shutdown and injected into newly initialized service container after kernel boot during test execution.
  • client's kernel can be rebooted using $I->rebootClientKernel() during test execution.
  • permanent services are updated after test execution.
  • cached router is permanent by default if used.
  • doctrine's EM service is permanent by default.

Test results of Codeception's Symfony2 framework test app (https://github.com/Codeception/symfony-demo):
symfony2_test_app

Test results of modified version of https://github.com/AlexStansfield/codeception-test - an application used to reproduce #2025:
issue2025_test_app
Modifications to this test app include:

  • creation of a copy of EntityCest.php - Entinty1Cest.php to check several cests in a suite,
  • making some methods of EntityCest.php to perform several sendPOST(...) requests to check correct work and measure performance of a solution.

Performing tests with modified version of Codeception on my Symfony2 app showed memory consumption was around 190 Mb's (previously tests failed due to insufficient memory with a 3GB's configuration) and execution time became around 19 minutes (and even lower - 11 minutes - with config['cached_router'] = true)

This PR also includes:

}

/**
* @param array $services
* Reboot kernel
*

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace found at end of line

@vyarmolenko vyarmolenko force-pushed the symfony2-update branch 4 times, most recently from 064a2ae to b16142e Compare April 7, 2016 20:08
$result = null;
if (property_exists($this, $property)) {
switch ($property) {
case 'container':

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace found at end of line

@raistlin
Copy link
Member

raistlin commented Apr 8, 2016

You have removed my lasts commit, one that deprecated grabServiceFromContainer and introduces a new grabService.
Could you rebase please?

@raistlin
Copy link
Member

raistlin commented Apr 8, 2016

How your modifications behave with current situation:

  • You make a request that grab some data from Doctrine2, update entities or even create new entities and call persist but you do not call to flush
  • You make other request. Will your second request see modifications done on first one?

@vyarmolenko
Copy link
Contributor Author

There is no need to deprecate grabServiceFromContainer with proposed changes. The service is being retrieved from the appropriate container transparently.

We have persistent Entity Manager and Unit Of Work and in described situation all persist'ed entities become managed and preserved between requests. However as stated in doctrine/orm#5092 unflushed entities cannot be accessed in common way (eg something like$entity = $em->find('AppBundle:Codeception', $name) will return null for unflushed entity).
So the answer is yes, the second request can see modifications done by first one but with some restrictions (and it is a Doctrine issue, not Codeception's). The behaviour is the same as within one request.

@raistlin
Copy link
Member

raistlin commented Apr 8, 2016

Rename of grabServiceFromContainer has been done to be consistent with other frameworks, that also have grabService.

@vyarmolenko
Copy link
Contributor Author

Updated, please review.

*/
public $container;
private $container;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This var is not used, and is a breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make $container a read-only property and retrieve it with magic method __get($property) without breaking BC

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, you can do it to handle the BC on 2.1.x. We should remove this 'magic' in a future release, maybe 3.0, and have only _getContainer.

@@ -67,11 +70,11 @@
* * container - dependency injection container instance
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last change: As this property no longer exists as is, please remove it from documentation.

@vyarmolenko vyarmolenko force-pushed the symfony2-update branch 4 times, most recently from bb34fbe to 74621f5 Compare April 11, 2016 17:38
…, update Doctrine EM retrieval. Improve memory usage. Unify service retrieval.
@raistlin
Copy link
Member

👍
Status: Reviewed.

@DavertMik
Copy link
Member

Thanks, awesome work!

@DavertMik DavertMik merged commit eedd32c into Codeception:2.1 Apr 12, 2016
raistlin added a commit that referenced this pull request Apr 12, 2016
Updated changelog adding changes made by #2968
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