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

[Issue] [TestFramework] Stopped marking transaction in integration tests as inactive although they're still active #39460

Open
2 of 5 tasks
m2-assistant bot opened this issue Dec 10, 2024 · 6 comments · May be fixed by #39429
Open
2 of 5 tasks
Labels
Area: Test framework Component: Framework/TestFramework Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: PR in progress Reported on 2.4.x Indicates original Magento version for the Issue report. Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it

Comments

@m2-assistant
Copy link

m2-assistant bot commented Dec 10, 2024

This issue is automatically created based on existing pull request: #39429: [TestFramework] Stopped marking transaction in integration tests as inactive although they're still active


Description (*)

Before an integration test is started the testing framework begins a new database transaction so that all changes to the database can be reverted after the test is completed.

The testing framework also offers a way to easily load data fixtures during this transaction with the help of the Magento\TestFramework\Fixture\DataFixture attribute. But in case of faulty fixtures an exception or error can occur. Currently this will prevent the endTest event from rolling back the transaction and so the tests fail with the error message:

Fatal error: Some transactions have not been committed or rolled back in /var/www/html/vendor/magento/framework/DB/Adapter/Pdo/Mysql.php on line 4169

instead of showing the real reason of the error.

The reason why the transaction is not rolled back correctly is that the catch block in the Transaction event marks the transaction as 'inactive' (see here) so that it won't be rolled back at the end of the test (see here), although the transaction is still open.

This PR fixes this behaviour so that the transaction is rolled back after the test execution and the correct error messages is shown in the console output.

Manual testing scenarios (*)

Add an integration test to your own test suite that uses a data fixture with an invalid config. For example:

<?php

namespace MyVendor\MyModule\Test\Integration;

use Magento\Catalog\Test\Fixture\Product as ProductFixture;
use Magento\TestFramework\Fixture\DataFixture;
use Magento\TestFramework\Fixture\DbIsolation;
use PHPUnit\Framework\TestCase;

class SynchronizerTest extends TestCase
{
    #[DbIsolation(true)]
    #[DataFixture(
        ProductFixture::class,
        ['sku' => 'test', 'name' => 'Test', 'price' => '7.77'],
        'test',
        'invalid_scope_key' // this line results in an error
    )]
    public function testSynchronize(): void
    {
        // can be empty
    }
}

Now, run the test and you should get this output:

PHPUnit 9.6.21 by Sebastian Bergmann and contributors.

FRolled back transaction has not been completed correctly.

=== Memory Usage System Stats ===
Memory usage (OS): 91.20M (188.03% of 48.50M reported by PHP)
Estimated memory leak: 42.70M (46.82% of used memory)

Fatal error: Some transactions have not been committed or rolled back in /var/www/html/vendor/magento/framework/DB/Adapter/Pdo/Mysql.php on line 4169

After applying this PR the output of phpunit shows the following:

PHPUnit 9.6.21 by Sebastian Bergmann and contributors.

F.. 3 / 3 (100%)

Time: 00:18.394, Memory: 48.00 MB

There was 1 failure:

  1. MyVendor\MyModule\Test\Integration\SynchronizerTest::testSynchronize
    PHPUnit\Framework\Exception: Unable to apply fixture: Magento\Catalog\Test\Fixture\Product (test)
    #0 /var/www/html/app/code/MyVendor/MyModule/Test/Integration/Synchronizer.php(51): MyVendor\MyModule\Test\Integration\SynchronizerTest->testSynchronize()

Caused By: ... rest of the exception with backtrace

This helps the developer to find the real issue. The previous message 'Some transactions have not been committed or rolled back' does not help.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)
@github-project-automation github-project-automation bot moved this to Ready for Confirmation in Issue Confirmation and Triage Board Dec 10, 2024
@engcom-Bravo engcom-Bravo added Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it Reported on 2.4.x Indicates original Magento version for the Issue report. labels Dec 10, 2024
@engcom-Hotel engcom-Hotel self-assigned this Dec 11, 2024
Copy link
Author

m2-assistant bot commented Dec 11, 2024

Hi @engcom-Hotel. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

  • 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
  • 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue.
  • 3. Add Area: XXXXX label to the ticket, indicating the functional areas it may be related to.
  • 4. Verify that the issue is reproducible on 2.4-develop branch
    Details- If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
  • 5. Add label Issue: Confirmed once verification is complete.
  • 6. Make sure that automatic system confirms that report has been added to the backlog.

@engcom-Hotel
Copy link
Contributor

Hello @stollr,

It seems this issue is a duplicate of #39463. I think we can close this one as the other issue is already confirmed.

Thanks

@engcom-Hotel engcom-Hotel added Issue: needs update Additional information is require, waiting for response and removed Issue: ready for confirmation labels Dec 11, 2024
@engcom-Hotel engcom-Hotel moved this from Ready for Confirmation to Needs Update in Issue Confirmation and Triage Board Dec 11, 2024
@stollr
Copy link
Contributor

stollr commented Dec 12, 2024

Hi @engcom-Hotel,
this is an independent issue, which already exists in the current stable release (2.4.7-p3), with phpunit 9.6.

But the issue in #39463 makes it impossible to reproduce the error of this issue in the current 2.4-develop branch. To reproduce it, please comment out this line:

Then you will be able to reproduce the issue and see the error message that I have quoted.

Btw. there is a pull request to fix this issue.

I have reported multiple issues, which have to do with the integration test framework, which may be confusing... sorry for that ;-)

@engcom-Hotel
Copy link
Contributor

Hello @stollr,

Thanks for the information!

We have tried to reproduce the issue and it is reproducible for us. Please refer to the below screenshot for reference:

image

Hence confirming the issue.

Thanks

@engcom-Hotel engcom-Hotel added Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Component: Framework/TestFramework Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Area: Test framework and removed Issue: needs update Additional information is require, waiting for response labels Dec 13, 2024
@github-jira-sync-bot
Copy link

✅ Jira issue https://jira.corp.adobe.com/browse/AC-13523 is successfully created for this GitHub issue.

Copy link
Author

m2-assistant bot commented Dec 13, 2024

✅ Confirmed by @engcom-Hotel. Thank you for verifying the issue.
Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Test framework Component: Framework/TestFramework Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: PR in progress Reported on 2.4.x Indicates original Magento version for the Issue report. Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
None yet
4 participants