Skip to content

2.2.4 : Magento 2 integration tests enables all modules #15196

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

Closed
kanduvisla opened this issue May 14, 2018 · 18 comments
Closed

2.2.4 : Magento 2 integration tests enables all modules #15196

kanduvisla opened this issue May 14, 2018 · 18 comments
Labels
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 Progress: PR Created Indicates that Pull Request has been created to fix issue 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

@kanduvisla
Copy link
Contributor

By default, I disable a lot of modules in my Magento setup, simply because I don't need them and I wish to spare my resources for the modules that I do need.

But I am currently in the situation where my integration test suite always runs with all modules enabled, whereas I want the suite to run again the same configuration is my store (so with a bunch of modules disabled).

A simple example: I don't use the Vertex_Tax-module so I have disabled it, but now my integration test suite is failing because some interceptors of this module expect some data to be populated:

Error: Call to undefined method Magento\Customer\Api\Data\CustomerExtension::setVertexCustomerCode()

Preconditions

  1. Install Magento 2.2.4

Steps to reproduce

  1. Disable some modules
  2. Run the integration test suite (or run your own)

Expected result

The integration test suite should respect the enabled / disabled module status.

Actual result

The integration test suite enables all modules.

@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label May 14, 2018
@kanduvisla
Copy link
Contributor Author

As a (hopefully temporary) workaround for this I came up with a solution where I created a wrapper around the bootstrap (call it a "bootwrap" if you may).

So in my phpunit.xml, I set my boostrap to:

bootstrap="./etc/bootstrap.php"

And etc/bootstrap.php looks like this:

// Bootstrap wrapper, used to disable certain modules for integration tests.
$ds = DIRECTORY_SEPARATOR;

// Default bootstrap, as provided by Magento:
include_once __DIR__ . $ds . implode($ds, ['..', 'framework', 'bootstrap.php']);

// Copy the original config.php, that has all the disabled modules:
$tmpDir = \Magento\TestFramework\Helper\Bootstrap::getInstance()->getAppTempDir();
$dir = __DIR__ . $ds . implode($ds, ['..', '..', '..', '..', 'app', 'etc']);
copy(
    $dir . $ds . 'config.php',
    $tmpDir . $ds . implode($ds, ['etc', 'config.php'])
);

Basically, the only thing it does, is copy your app/etc/config.php to your dev/tests/integration/tmp/xxxxxx/etc-folder; effectively disabling the proper modules.

@jissereitsma
Copy link
Contributor

This issue actually leads to a bigger issue. Within the phpunit.xml.dist configuration, there is the suggestion for using TESTS_GLOBAL_CONFIG_FILE which allows you to add new configuration options to your testing environment. However, the default options in the supplied config-global.php.dist file are never merged into the sandbox environment. Either, the whole mentioning of TESTS_GLOBAL_CONFIG_FILE needs to be removed, or it needs to be fixed. I'll have a PR for this in an hour or so.

Once this fix is included in Magento, you can create a new file dev/tests/integration/etc/config-global.php where you can include the exact contents of your global app/etc/config.php. Or you can use a custom bootstrap like the following to actually merge the global app/etc/config.php and the config-global.php.dist supplied by Magento. Thanks @kanduvisla for pointing the option for a custom bootstrap. My approach is a bit different, because it assumes the fix (PR coming) and therefore only generates a fresh config-global.php which should be considered as optional.

My bootstrap etc/custom-bootstrap.php is referenced in PHPUnit as such:

bootstrap="./etc/custom-bootstrap.php"

The bootstrap file itself looks like this:

<?php
$defaultConfig = include __DIR__ . '/config-global.php.dist';
$actualConfig = include __DIR__ . '/../../../../app/etc/config.php';
$newConfig = array_merge($defaultConfig, $actualConfig);

$code = var_export($newConfig, true);
$code = "<?php\n return " . preg_replace('/stdClass::__set_state/', '(object)', $code) . ';';
file_put_contents(__DIR__ . '/config-global.php', $code);

include_once __DIR__ . '/../framework/bootstrap.php';

@jissereitsma
Copy link
Contributor

PR created: #16361

@magento-engcom-team magento-engcom-team added the Progress: PR Created Indicates that Pull Request has been created to fix issue label Jul 5, 2018
@kanduvisla
Copy link
Contributor Author

kanduvisla commented Jul 11, 2018

It turns out my little trick no longer works (perhaps the 2.2.5 update caused it to stop working).

Isn't there any way to disable certain modules for running integration tests on a local development setup? Anyone? Already spent way too much time on this one.

@magento-engcom-team : perhaps add an extra flag to phpunit.xml to opt-in for automatically enable all the modules for your integration tests? I just want my integration tests to have the same modules enabled as my production site (which makes sense)

edit: I checked my Git log and indeed: my little trick stopped working since the 2.2.5 update

@kanduvisla
Copy link
Contributor Author

So I seem to have "fixed" it once again with yet another dirty hack. Instead of having to copy/paste the original config.php before my tests, I had to dig deeper into the bootstrapping-context.

So I copied the original framework/bootstrap.php to my etc.php and edited as follows:

  • I changed the require_once __DIR__ ... -statements to require_once __DIR__ . '/../framework' so all required files got loaded properly.
  • I replaced the $application-instantiating as follows:

Original:

$application = new \Magento\TestFramework\Application(
    $shell,
    $installDir,
    $installConfigFile,
    $globalConfigFile,
    $settings->get('TESTS_GLOBAL_CONFIG_DIR'),
    $settings->get('TESTS_MAGENTO_MODE'),
    AutoloaderRegistry::getAutoloader(),
    true
);

My version:

// Manipulate existing application class to inject the projects' config.php:
$application = new class(
    $shell,
    $installDir,
    $installConfigFile,
    $globalConfigFile,
    $settings->get('TESTS_GLOBAL_CONFIG_DIR'),
    $settings->get('TESTS_MAGENTO_MODE'),
    AutoloaderRegistry::getAutoloader(),
    true
) extends \Magento\TestFramework\Application
{
    /**
     * @inheritDoc
     */
    public function install($cleanup)
    {
        $this->_ensureDirExists($this->installDir);
        $this->_ensureDirExists($this->_configDir);

        $file = $this->_globalConfigDir . '/config.php';
        $targetFile = $this->_configDir . str_replace($this->_globalConfigDir, '', $file);

        $this->_ensureDirExists(dirname($targetFile));
        if ($file !== $targetFile) {
            copy($file, $targetFile);
        }

        parent::install($cleanup);
    }

    /**
     * @inheritDoc
     */
    public function isInstalled()
    {
        // Always return false, otherwise DB credentials will be empty
        return false;
    }
};

I had to use an anonymous class because the new instantiation of the Application prevented me from using proper dependency injection. What it basically does, is it copies the config.php-file before the original install()-method is called. So when a native installation starts, it uses the config.php of my project to determine which modules should be installed.

Also, I had to override the isInstalled()-method to always return false, because otherwise the database function would have the wrong credentials (isInstalled() would always return true if a config.php is found).

So this way the integration tests on our CI server will run on an installation that reflects our production setup.

@engcom-backlog-nickolas engcom-backlog-nickolas added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development 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 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 Jul 13, 2018
@sidolov
Copy link
Contributor

sidolov commented Sep 22, 2018

Hi @kanduvisla. Thank you for your report.
The issue has been fixed in #16361 by @jissereitsma in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming 2.2.8 release.

@sidolov sidolov added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Sep 22, 2018
@sidolov sidolov closed this as completed Sep 22, 2018
@slavvka slavvka added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Oct 3, 2018
@slavvka
Copy link
Member

slavvka commented Oct 3, 2018

Hi @kanduvisla. Thank you for your report.
The issue has been fixed in #18201 by @mage2pratik in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.1 release.

@JosephLeedy
Copy link

JosephLeedy commented Feb 21, 2019

@mage2pratik's patch seems to be somewhat broken. It copies the config file successfully, but the defaults-extra.cnf file does not generate correctly because the test runner seems to no longer get the database credentials from install-config-mysql.php when generating it. If I put the MySQL credentials into config-global.php, it works correctly. It's worth noting that the database is actually created fine and used up to that point, so that seems to be the only part affected.

Environment:
OS: MacOS 10.14.3
Web Server: Nginx 1.15.8
PHP: PHP 7.2.15
MySQL: MariaDB 10.3.12
Magento: Open Source 2.3.0

@Tjitse-E
Copy link
Contributor

I had the same thing with Magento 2.3.2. What i've done is:

  1. Change the isInstalled() function like this:
     public function isInstalled()
     {
        return false;
     }

Then change the copyGlobalConfigFile() to:

     private function copyGlobalConfigFile()
     {
        $targetFile = $this->_configDir . '/config.php';
         copy($this->globalConfigFile, $targetFile);
     }

When isInstalled() returns true, than the defaults_extra.cnf will contain empty values. And i'm not sure that the config.local.php file does anything. Now everything I place in etc/config-global.php will be included in the test.

@bunyamin-gc
Copy link
Contributor

bunyamin-gc commented Apr 15, 2020

Hi @Tjitse-E ,

I have confirmed that config.local.php does nothing. When the sandbox environment loads configurations, it does not read config.local.php file.

@schmengler
Copy link
Contributor

schmengler commented Aug 28, 2020

Confirmed as well:

In \Magento\Framework\Config\File\ConfigFilePool::getPathsByPool():

    /**
     * Retrieve all config file pools.
     *
     * @param string $pool
     * @return array
     * @deprecated 101.0.0 Magento does not support custom config file pools since 2.2.0 version
     * @since 100.1.3
     */
    public function getPathsByPool($pool)
    {
        return $this->initialConfigFiles[$pool];
    }

This is the method that returns config.local.php and it is not used anywhere anymore.

However, we found a different solution:

In install-config-mysql.php, add the following parameter with a comma separated list of modules

  'disable-modules' => '...',

@jissereitsma
Copy link
Contributor

@schmengler Haha, we should bundle these resources. Because I've been using disable-modules in my install scripts for some time. Even with a custom made function to "calculate" which modules to enable and which ones not.

@schmengler
Copy link
Contributor

@jissereitsma Nice, would you share it here? I guess it reads config.php and of course we can build our own, but I'll not be the last to come here and look for a solution :)

@jissereitsma
Copy link
Contributor

@schmengler Argh, just looked it up but I threw it out. I'm currently relying on composer replace, combined with MySQL in tmpfs, combined with ReachDigitals framework. Anyway, the function simply did an include of the original app/etc/config.php its modules section, looking through the modules and matching them by name: Including my own modules, including modules starting with Magento_ but excluding then specific others (*Adobe*, *Inventory*, etc).

@t-heuser
Copy link

t-heuser commented Dec 7, 2020

This issue still exists in Magento 2.4.1. Why is this closed?
Had to use @kanduvisla fix which fortunately still works.

@lbajsarowicz
Copy link
Contributor

lbajsarowicz commented Jul 16, 2021

If I understand the issue correctly, it is still there (2.4.2) - I am unable to use custom TESTS_GLOBAL_CONFIG_FILE to adjust the settings for tests. What I need to achieve is the possibility to use getenv or $ENV in the system configuration (What was previously done in config-global.php)

@PascalBrouwers
Copy link
Contributor

Come here to disable modules during integration tests too.
Came across the disable-modules option in install-config-mysql.php using:

'disable-modules' => join(
        ',',
        [
            'My_Module',
        ]
    ),

@lbajsarowicz
Copy link
Contributor

lbajsarowicz commented Mar 12, 2023

@PascalBrouwers As config.php is a part of application -- You have permanent (locked) application configurations, I don't understand why Magento employees ("engineers") decided to exclude config.php from being a part of the Integration Tests Suite.

As I'm strongly disappointed with the approach (and I lock some configurations, to make sure Client won't cut their hands playing with the configuration), I created a Composer Patch to magento/base

diff --git a/dev/tests/integration/framework/Magento/TestFramework/Application.php b/dev/tests/integration/framework/Magento/TestFramework/Application.php
--- a/dev/tests/integration/framework/Magento/TestFramework/Application.php
+++ b/dev/tests/integration/framework/Magento/TestFramework/Application.php
@@ -293,8 +293,12 @@
      */
     public function isInstalled()
     {
         // phpcs:ignore Magento2.Functions.DiscouragedFunction
-        return is_file($this->getLocalConfig());
+        $envFile = $this->_configDir . '/env.php';
+        if (is_file($envFile)) {
+            $envConfig = include $envFile;
+            return $envConfig['install']['date'] ?? false;
+        }
     }

     /**
@@ -547,7 +547,7 @@
     private function copyAppConfigFiles()
     {
         $globalConfigFiles = Glob::glob(
-            $this->_globalConfigDir . '/{di.xml,*/di.xml,db_schema.xml,vendor_path.php}',
+            $this->_globalConfigDir . '/{di.xml,*/di.xml,db_schema.xml,vendor_path.php,config.php}',
             Glob::GLOB_BRACE
         );
         foreach ($globalConfigFiles as $file) {

So, if you have disabled a module in config.php, when executing Integration Tests, you receive consistent results whether you test the Storefront, Admin or... executing Test Suite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 Progress: PR Created Indicates that Pull Request has been created to fix issue 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