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

Unbind specific event handler instead of all class level events when … #5045

Merged
merged 5 commits into from
Jun 28, 2018

Conversation

SamMousa
Copy link
Collaborator

@SamMousa SamMousa commented Jun 28, 2018

This PR refactors database connection handling for the Yii2 module.

It is fully backwards compatible with the exception of broken stuff that is now fixed.

Database connections should now always be closed after tests no matter how you have opened them or who is holding references to them.


public function __construct()
{
$this->handler = function(Event $event) {

Choose a reason for hiding this comment

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

Expected 1 space after FUNCTION keyword; 0 found

{
$count = count($this->connections);
$this->debug("closing all ($count) connections");
foreach($this->connections as $connection) {

Choose a reason for hiding this comment

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

Expected 1 space after FOREACH keyword; 0 found

}
codecept_debug("[$title] $message");
}
}

Choose a reason for hiding this comment

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

Expected 1 newline at end of file; 0 found

<?php


namespace Codeception\Lib\Connector\Yii2;

Choose a reason for hiding this comment

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

There must be one blank line after the namespace declaration

$this->pdoCache = [];
$this->dsnCache = [];
}
}

Choose a reason for hiding this comment

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

Expected 1 newline at end of file; 0 found

@SamMousa
Copy link
Collaborator Author

@samdark This is ready for merge.

After it has been merged I will also merge this one: Codeception/yii2-tests#2 which tests the proper closing of database connection in most (if not all) scenarios.

@samdark samdark merged commit c04934d into Codeception:2.4 Jun 28, 2018
@samdark
Copy link
Collaborator

samdark commented Jun 28, 2018

Merged. Thanks!

@SamMousa SamMousa deleted the yii2-event-handlers branch June 28, 2018 15:13
@SamMousa SamMousa restored the yii2-event-handlers branch June 28, 2018 15:13
@SamMousa SamMousa deleted the yii2-event-handlers branch June 28, 2018 15:14
@roslov
Copy link
Contributor

roslov commented Jul 16, 2018

Nope :-(
It still does not work:
Too many connections

@SamMousa
Copy link
Collaborator Author

I can't explain this, there's a testcase for it that succeeds..

https://travis-ci.org/Codeception/yii2-tests/builds/397866937

@roslov
Copy link
Contributor

roslov commented Jul 16, 2018

Probably that test does not cover all cases. I do not know why, but on 2.4.0 and previous versions everything works fine.

@SamMousa
Copy link
Collaborator Author

SamMousa commented Jul 16, 2018

I do not know why, but on 2.4.0 and previous versions everything works fine.

It worked fine in your specific case (single database component named db). In all other cases it did not work at all.

Probably that test does not cover all cases.

@roslov could you make a PR with a minimal test case where you can reproduce it?

@githubjeka
Copy link
Contributor

After this fix I can't run tests:

Modules: Yii2, Asserts, \businessTest\Helper\Unit
--------------------------------------------------------------------------------------------------------------------------------------------------------
E AuthEmployeeTest: True handle 
E AuthEmployeeTest: True handle (0.02s)
PHP Fatal error:  Unc
Fatal error: Uncaught Error: Call to a member function stop() on null in \vendor\codeception\codeception\src\Codeception\Module\Yii2.php:346

Stack trace:
#0 vendor\codeception\codeception\src\Codeception\Subscriber\Module.php(66): Codeception\Module\Yii2->_after(Object(businessTest\unit\AuthEmployeeTest))
#1 vendor\symfony\event-dispatcher\EventDispatcher.php(212): Codeception\Subscriber\Module->after(Object(Codeception\Event\TestEvent), 'test.after', Object(Symfony\Component\EventDispatcher\EventDispatcher))
#2 vendor\symfony\event-dispatcher\EventDispatcher.php(44): Symfony\Component\EventDispatcher\EventDispatcher->doDispatch(Array, 'test.after', Object(Codeception\Event\TestEvent))
#3vendor\codeception\phpunit-wrapper\src\Listener.php(133): Symfony\Component\EventDispatcher\EventDispatcher->dispatch('test.after' in vendor\codeception\codeception\src\Codeception\Module\Yii2.php:346

@SamMousa
Copy link
Collaborator Author

That's weird; it is created on _before() and only cleared in _after().
Please create a PR so we can reproduce the error here: https://github.com/codeception/yii2-tests

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.

5 participants