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

Print debug information for started/stopped transactions in tests #4352

Merged
merged 2 commits into from Jul 7, 2017

Conversation

Projects
None yet
3 participants
@DavertMik
Member

DavertMik commented Jun 20, 2017

Following the proposal from @ThomasLandauer #4326 (comment)
I added debug notifications when transaction is started for tests.

Probably the message should be improved, so suggestions are welcome

Show outdated Hide outdated src/Codeception/Module/Laravel5.php
@@ -175,6 +175,8 @@ public function _before(\Codeception\TestInterface $test)
if ($this->applicationUsesDatabase() && $this->config['cleanup']) {
$this->app['db']->beginTransaction();
$this->debugSection('Transaction', 'Started');

This comment has been minimized.

@Naktibalda

Naktibalda Jun 21, 2017

Member

Please delete this blank line.

@Naktibalda

Naktibalda Jun 21, 2017

Member

Please delete this blank line.

@ThomasLandauer

This comment has been minimized.

Show comment
Hide comment
@ThomasLandauer

ThomasLandauer Jun 21, 2017

Contributor

Please commit, so I can see the message "live" :-)

Contributor

ThomasLandauer commented Jun 21, 2017

Please commit, so I can see the message "live" :-)

@Naktibalda

This comment has been minimized.

Show comment
Hide comment
@Naktibalda

Naktibalda Jun 21, 2017

Member

@ThomasLandauer checkout debug-transactions branch from this repo.

Member

Naktibalda commented Jun 21, 2017

@ThomasLandauer checkout debug-transactions branch from this repo.

@ThomasLandauer

This comment has been minimized.

Show comment
Hide comment
@ThomasLandauer

ThomasLandauer Jun 21, 2017

Contributor

Well, I switched to this branch, downloaded the ZIP file and copied it manually into vendor/codeception. Would there have been an easier/better way?

Anyway, I can't see any difference. Am I looking in the right place? ;-) I expected a new line in the output, somewhere in here:

Acceptance Tests (1) ---------------------------------------------------------------------------------------------------
Modules: Symfony, Doctrine2, WebDriver, Asserts, \Helper\Acceptance
------------------------------------------------------------------------------------------------------------------------
Testing acceptance
Contributor

ThomasLandauer commented Jun 21, 2017

Well, I switched to this branch, downloaded the ZIP file and copied it manually into vendor/codeception. Would there have been an easier/better way?

Anyway, I can't see any difference. Am I looking in the right place? ;-) I expected a new line in the output, somewhere in here:

Acceptance Tests (1) ---------------------------------------------------------------------------------------------------
Modules: Symfony, Doctrine2, WebDriver, Asserts, \Helper\Acceptance
------------------------------------------------------------------------------------------------------------------------
Testing acceptance
@Naktibalda

This comment has been minimized.

Show comment
Hide comment
@Naktibalda

Naktibalda Jun 21, 2017

Member

Do you run it with -vv flag?

Member

Naktibalda commented Jun 21, 2017

Do you run it with -vv flag?

@ThomasLandauer

This comment has been minimized.

Show comment
Hide comment
@ThomasLandauer

ThomasLandauer Jun 21, 2017

Contributor

Yes. What would the output be supposed to look like?

Contributor

ThomasLandauer commented Jun 21, 2017

Yes. What would the output be supposed to look like?

@Naktibalda

This comment has been minimized.

Show comment
Hide comment
@Naktibalda

Naktibalda Jun 21, 2017

Member

[Transaction] Started

Member

Naktibalda commented Jun 21, 2017

[Transaction] Started

@ThomasLandauer

This comment has been minimized.

Show comment
Hide comment
@ThomasLandauer

ThomasLandauer Jun 21, 2017

Contributor

Well, maybe I didn't see it cause I had cleanup: false... ;-)))

Two suggestions:

  1. Reword it to: Database: Transaction startet and Database: Transaction stopped; all changes reverted. Reason: Give more context (i.e. "database") to indicate what sort of "transaction" we are talking about.

  2. More important: Give this information in the header line too, namely the true as well as the false case, like this:

Acceptance Tests (1) ---------------------------------------------------------------------------------------------------
Modules: Symfony, Doctrine2, WebDriver, Asserts, \Helper\Acceptance
Doctrine2 is in transaction rollback mode (cleanup: true)
Doctrine2 is in non-transaction mode (cleanup: false)
------------------------------------------------------------------------------------------------------------------------
Testing acceptance
Contributor

ThomasLandauer commented Jun 21, 2017

Well, maybe I didn't see it cause I had cleanup: false... ;-)))

Two suggestions:

  1. Reword it to: Database: Transaction startet and Database: Transaction stopped; all changes reverted. Reason: Give more context (i.e. "database") to indicate what sort of "transaction" we are talking about.

  2. More important: Give this information in the header line too, namely the true as well as the false case, like this:

Acceptance Tests (1) ---------------------------------------------------------------------------------------------------
Modules: Symfony, Doctrine2, WebDriver, Asserts, \Helper\Acceptance
Doctrine2 is in transaction rollback mode (cleanup: true)
Doctrine2 is in non-transaction mode (cleanup: false)
------------------------------------------------------------------------------------------------------------------------
Testing acceptance
@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Jun 21, 2017

Member
  1. OK, I asked for better texting so yes, good suggestion, I will apply
  2. No, output becomes too verbose
Member

DavertMik commented Jun 21, 2017

  1. OK, I asked for better texting so yes, good suggestion, I will apply
  2. No, output becomes too verbose
@ThomasLandauer

This comment has been minimized.

Show comment
Hide comment
@ThomasLandauer

ThomasLandauer Jun 21, 2017

Contributor

Are you kidding? That's just one (!) line more of debug output, which is (probably) several hundreds of lines long anyway!

We could "trade" it for another line, which is superflous (seriously):

Codeception PHP Testing Framework v2.3.3
Powered by PHPUnit 5.7.20 by Sebastian Bergmann and contributors.

Functional Tests (26) --------------------------------------------------------------------------------------------------
Testing functional

The last line ("Testing functional") should be removed, since it doesn't give any new information.

Contributor

ThomasLandauer commented Jun 21, 2017

Are you kidding? That's just one (!) line more of debug output, which is (probably) several hundreds of lines long anyway!

We could "trade" it for another line, which is superflous (seriously):

Codeception PHP Testing Framework v2.3.3
Powered by PHPUnit 5.7.20 by Sebastian Bergmann and contributors.

Functional Tests (26) --------------------------------------------------------------------------------------------------
Testing functional

The last line ("Testing functional") should be removed, since it doesn't give any new information.

@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Jun 27, 2017

Member

Doctrine2 is in transaction rollback mode (cleanup: true)

Sorry module doesn't have access to output other than debug so it can't print messages like this one.

I changed wording. If everything ok, @Naktibalda please merge it

Member

DavertMik commented Jun 27, 2017

Doctrine2 is in transaction rollback mode (cleanup: true)

Sorry module doesn't have access to output other than debug so it can't print messages like this one.

I changed wording. If everything ok, @Naktibalda please merge it

@DavertMik DavertMik merged commit 0f3ac29 into 2.3 Jul 7, 2017

6 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details
wercker/build Wercker pipeline passed
Details

@DavertMik DavertMik deleted the debug-transactions branch Jul 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment