We need a way to load and check for dependencies as well to enable contributed modules in the test case implementations.

CommentFileSizeAuthor
#9 1845978.9.patch6.2 KBGDrupal
#3 1845978.3.patch2.09 KBGDrupal
#1 1845978.1.patch1.84 KBGDrupal

Comments

GDrupal’s picture

GDrupal commented
Status: Active » Needs review
StatusFileSize
new 1845978.1.patch1.84 KB
GDrupal’s picture

GDrupal commented
Status: Needs review » Fixed

Committed

GDrupal’s picture

GDrupal commented
Status: Fixed » Needs review
StatusFileSize
new 1845978.3.patch2.09 KB

Add support dependencies to the export test method.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Status: Needs review » Needs work 
+++ b/tests/handlers/ConfigurationHandlerBaseTestCase.test
@@ -183,6 +191,13 @@ abstract class ConfigurationHandlerBaseTestCase extends ConfigurationWebTestCase
+  /**
+   * Returns an array of configurations to check if they were exported.
+   */
+  protected function dependenciesToExport() {
+    return FALSE;
+  }

If the docs says return an array, you shouldn't return FALSE.

Also, I think this patch is not correct. What is the implementation of dependenciesToExport? The iterate method is who proccess this dependencies.

GDrupal’s picture

GDrupal commented

Since we are perhaps creating the object to test export manually... we cant use the same list as import dependencies.. its the same idea that the previous patch that is already working only to separate the list of dependencies to check for import /export test cases. This is already working in my local, i'm just trying to make the patch smaller to concentrate in other bugs I have found.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented

I understand that, however that function provides other way to do the same. Also I see this more like a hack than a testing function. Maybe if you show the original issue we can find an elegant solution without introduce this hack.

GDrupal’s picture

GDrupal commented

It's not a hack... simply a wrapper to provide a custom list of dependencies to check... so when you export a component.. the already existing test method looks for that components too....it's doing exactly the same as configToImport() but for providing a list of dependencies... I think you have misunderstood the patch.

The previous patch provided "dependenciesToCheck()" doing something similar that "configToImport()" but for dependencies...

This patch expand that patch so now we have:

  • dependenciesToImport() to add dependencies to check for as configToImport()
  • dependenciesToExport() to add dependencies to check for as dependenciesToExport()

So we can also reuse all the logic in testExportToDataStore() checking for dependencies if we provide a list of them...

GDrupal’s picture

GDrupal commented

In fact I just realized that we need another method in the same way to add a list of optional components to check for too.

GDrupal’s picture

GDrupal commented
Status: Needs work » Needs review
StatusFileSize
new 1845978.9.patch6.2 KB
GDrupal’s picture

GDrupal commented
Status: Needs review » Fixed

Committed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.