array_unique notice from List unmet configuration dependencies instead of just failing

What about introducing some small test to it?

@GoZ I think you're taking the right approach here. Someone else wiser than me might think otherwise, but I agree with you. The array_unique() usage looks to be left over from an early version of the other issue's patch in #2655104-13: List unmet configuration dependencies instead of just failing, which dealt with a flat array of dependencies, but it later changed into a multi-dimensional array which would cause this notice.

Looks good to me and the tests pass/fail as they should - thanks!

Hmm I think I would have gone for option 2 or 3 here to reduce the duplicates. Why not just use SORT_REGULAR to at least have fewer dups?

Here's a new patch then that uses the sort flag with array_unique(). :-)

Sorry patch #13 was rushed -- the change to use the sort flag was implemented, but I didn't change the test, which of course now needs to test for a different message.

Corrected the test.

+++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigInstallTest.php
@@ -202,6 +202,15 @@ public function testDependencyChecking() {
+      $this->assertEqual($e->getExtension(), 'config_install_double_dependency_test');
+      $this->assertEqual($e->getConfigObjects(), ['config_test.dynamic.other_module_test_with_dependency' => ['config_other_module_config_test', 'config_test.dynamic.dotted.english']]);
+      $this->assertEqual($e->getMessage(), 'Configuration objects provided by <em class="placeholder">config_install_double_dependency_test</em> have unmet dependencies: <em class="placeholder">config_test.dynamic.other_module_test_with_dependency (config_other_module_config_test, config_test.dynamic.dotted.english)</em>');

This is new code, we should not use deprecated assert methods: s/assertEqual/assertEquals.

IMHO we need to clean full core to remove the deprecated asset methods. There are many places where i saw that there are many places where these assert methods are used.

    \Drupal::service('config.manager')->uninstall('module', 'config_collection_install_test');
    $this->assertEqual(array('entity'), $active_storage->getAllCollectionNames());
    \Drupal::service('config.manager')->uninstall('module', 'config_test');
    $this->assertEqual(array(), $active_storage->getAllCollectionNames()); 

Let's create a follow up for this. What do you think?

@himanshu-dixit it would need doing at some point, yes, though it needs to be in the right branch to for the BC policy.

@claudiu.cristea Thanks, good spot. Putting back to needs work then.

If we are going to fix it for this issue. Here is the patch,

UPDATE: Sorry, didn't noticed #21 was posted before i was submitting the patch.

+++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigInstallTest.php
@@ -202,6 +202,15 @@ public function testDependencyChecking() {
+      $this->assertEquals($e->getExtension(), 'config_install_double_dependency_test');
+      $this->assertEquals($e->getConfigObjects(), ['config_test.dynamic.other_module_test_with_dependency' => ['config_other_module_config_test', 'config_test.dynamic.dotted.english']]);
+      $this->assertEquals($e->getMessage(), 'Configuration objects provided by <em class="placeholder">config_install_double_dependency_test</em> have unmet dependencies: <em class="placeholder">config_test.dynamic.other_module_test_with_dependency (config_other_module_config_test, config_test.dynamic.dotted.english)</em>');

Note: assertEquals should get the expected value as first parameter.

@dawehner Thanks for catching this, i forgot about it KernelTestBase. Here is the new patch, do you think this can get RTBC?

RTBC if passes.

Committed/pushed to 8.4.x and cherry-picked to 8.3.x, thanks! Untagging as string change since there's no string change here.

Attribution Added