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

Integration tests don't reset the database #10025

Closed
dersam opened this issue Jun 22, 2017 · 14 comments
Closed

Integration tests don't reset the database #10025

dersam opened this issue Jun 22, 2017 · 14 comments
Assignees
Labels
Event: MMNY17 Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@dersam
Copy link

dersam commented Jun 22, 2017

Running the integration tests will not reset the integration test database. This results in unexpected states in the test database, breaking custom tests.

This is not a "TESTS_CLEANUP" issue. It is enabled. All caches and generated code have been repeatedly cleared.
<const name="TESTS_CLEANUP" value="enabled"/>

Unclear if this is an issue with 2.1.7, but the database did reset fully in the past.

Preconditions

  1. Magento version 2.1.7

Steps to reproduce

  1. Manually modify some data in the integration test database (this assumes the integration tests have been run previously.
  2. Run the integration tests as shown in http://devdocs.magento.com/guides/v2.0/test/integration/integration_test_execution_cli.html

Expected result

  1. The manually added data should no longer be present, as all the tables should have been dropped and setup scripts re-run from scratch.

Actual result

  1. The custom data still exists, indicating the database was not deleted and replaced.
@dersam
Copy link
Author

dersam commented Jun 22, 2017

I didn't realize that the integration tests create a dump of the database on success, and automatically restores it after cleanup.

Even though it's uninstalling, the same database is going to be restored, which seems misleading when TESTS_CLEANUP is enabled. Is this the intended behaviour?

@orlangur
Copy link
Contributor

Yes, as restoring database snapshot is a much faster operation than reinstalling from scratch.

@dersam
Copy link
Author

dersam commented Jul 26, 2017

Doing it for performance makes sense, but if a dump is being restored, what is being cleaned up? If I have created database entries as part of my test run, these now exist in my dump, because the dump happens after the tests have run.

The next time I run my test, the database contains these old entries, which will then break any tests that want to create these entries.

Wouldn't it make more sense to create the dump immediately after the db install and upgrade scripts have run? That way no test data is included.

@magento-engcom-team magento-engcom-team added G1 Passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 5, 2017
@giacmir
Copy link
Member

giacmir commented Sep 14, 2017

I agee with @dersam. If a dump should be made (and I think it should not and all the system should be reinstalled every time, what if I changed a setup script because of a typo or something similar?) it should be created just after the setup, not at the end of all tests.

@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Sep 28, 2017
@magento-engcom-team
Copy link
Contributor

@dersam, thank you for your report.
We've created internal ticket(s) MAGETWO-80343 to track progress on the issue.

@magento-engcom-team magento-engcom-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Sep 28, 2017
@joshuaswarren
Copy link

joshuaswarren commented Oct 16, 2017

I've been looking into this issue as part of Squashtoberfest, and looking at 2.0, 2.1 and 2.2 (including 2.2-develop), the only place I can find in the integration test suite where the SQL database dump is being created is here: https://github.com/magento/magento2/blob/2.2-develop/dev/tests/integration/framework/Magento/TestFramework/Application.php#L487

This is at the end of the install function in the test suite.

Basically, the flow in the installation process for the integration tests is:

  1. Check for a DB dump - if it exists, import it
  2. Run the install process
  3. Create a DB dump

And the overall process of the integration tests (from https://github.com/magento/magento2/blob/2.2-develop/dev/tests/integration/framework/bootstrap.php) is:

  1. Read the settings & config files
  2. Bootstrap the Magento instance
  3. If the Magento instance isn't installed, install it (running the three steps above - creating the database dump at this point)
  4. Initialize Magento and run the tests

Based on this flow, the DB dump that's restored should only contain a snapshot of the database right after the Magento instance is installed and all install/setup scripts are run, but before the test scripts are run.

@dersam - is this not the behavior you're seeing? Can you let me know how and when you're adding new data to the test database?

FYI, I've tested this on 2.2.0 by running the integration tests, waiting until the first test starts to execute and then halting execution with CTRL+C. When I do this, the SQL dump file exists in dev/tests/integration/tmp/sandbox.../setup_dump_[name of database].sql and is a complete dump of the DB just after the setup scripts have completed.

@joshuaswarren
Copy link

Also, going back to @giacmir's comment (which appears to be a slightly separate issue - Dersam is reporting a problem where he has modified the test database during the integration test process and those modifications are being restored. Giacmir, you're pointing out the case where a problem might be in a setup script itself, so you'd prefer the DB not to be restored at all) - I think a good solution to that particular case would be to add an additional configuration variable in addition to TESTS_CLEANUP called TESTS_RESTORE_DB where TESTS_RESTORE_DB being set to true would use the current behavior and TESTS_RESTORE_DB set to false would ignore the code to restore the DB backup entirely.

@joshuaswarren
Copy link

It looks like the cleanest fix here is to change the behavior so that when TESTS_CLEANUP is enabled, the database dump is not restored.

@orlangur
Copy link
Contributor

So http://devdocs.magento.com/guides/v2.2/test/integration/integration_test_execution.html#the-testscleanup-constant is misleading and application reinstallation never occurs in fact?

@okorshenko
Copy link
Contributor

Hi @joshuaswarren please accept the invite to join magento2 repository

@joshuaswarren
Copy link

Thanks for the invite - accepted. I've submitted a pull request to resolve this issue as part of #SQUASHTOBERFEST

@okorshenko
Copy link
Contributor

The issue has been fixed in 2.2-develop branch

@okorshenko okorshenko added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Oct 24, 2017
@magento-team
Copy link
Contributor

Internal ticket to track issue progress: MAGETWO-82003

@magento-team
Copy link
Contributor

Hi @dersam. Thank you for your report.
The issue has been fixed in magento-engcom/magento2ce#1284 by @magento-engcom-team in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming patch release.

@magento-team magento-team added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Jan 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Event: MMNY17 Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

7 participants