Drush features-export doesn't create feature

Raising priority on this, the ability to create new features is a core functionality of Features and this is now broken.

Code review:

Coding standards

The patch is not compliant with coding standards, you can use a tool such as the Coder module in combination with PHP Codesniffer to easily detect these (once set up you can just do drush dcs [filename] to give you a list of code standards violations).

--- a/tests/features.test
+++ b/tests/features.test
@@ -280,3 +280,65 @@ class FeaturesCtoolsIntegrationTest extends DrupalWebTestCase {
     }
   }
 }
+
+
+
+/**

Adding multiple successive lines of whitespace. This also occurs in other places in the patch.

+class FeaturesDrushExportTest extends DrupalWebTestCase {
...
+      'description' => t('Run functional tests on Features `drush` commands.') ,

There is a space preceding the comma.

+class FeaturesDrushExportTest extends DrupalWebTestCase {
...
+  private $test_module_path = "sites/all/modules/my_sweet_feature";

Use single quotes instead of double quotes. There are also other instances where double quotes are used instead of single quotes.

+  public function setUp() {
+    parent::setUp(array('features',));
+  }

Don't use a trailing comma when defining an array on a single line.

+  public function testModuleExportNew(){

Precede curly braces with a space.

Typos

+  public function testModuleExportNew(){
+    // We need to make sure the module does not exists.

This should be '...does not exist'.

Functionality

In my install the test failed, and when I exported a new feature with a node type the module was created but was missing the node type export.

Hey, my review this morning was cut off abruptly by my train entering the station, sorry for the curtness! I was not actually using a review tool, I spotted these myself, I just wanted to suggest using a tool like coder or codesniffer :) Mind that these tools do not detect everything.

Indeed the problem you described in #1 is exactly what I encountered, just an empty skeleton module is created, without any exported components. It should not be necessary to enable the module on export to work around this issue since that might have unwanted side effects (hook_enabled() and hook_install() firing, ...).

Anyway I'm now on my next train journey, I will now review and test your updates. I think the test failures I encountered this morning might have to do with permissions on the sites/all/modules folder, will investigate and report back shortly.

I have been able to fix the failing test about the feature creation by saving the feature in the public:// folder. This works properly on installations that do not allow the web server to write in the sites/all/modules folder. This also has the advantage that the Simpletest creates a different unique folder with each test run, so we can get rid of the code that checks if the module already exists.

I also fixed some minor whitespace and double quotes issues, and improved some wording.

So the next thing is to figure out why the components are not exported.

Why would you need to load the module? The test should verify if the feature is exported correctly, and you can do that perfectly by looking at the exported files in the files folder.

Oh I see you have a call to features_load_feature() in the patch. That should not be there at all. If you are exporting a new feature it does not yet exist, so it cannot possibly be loaded. The web interface also allows to export modules and offer them as a tarred file to the browser, without placing files in the modules folder. Exporting with drush should be no different :)

Excellent work!! It works beautifully now.

I have made some minor changes to the patch as a final cleanup:

• Renamed _features_generate_export() to _drush_features_generate_export() to be consistent with other functions in this file.
• Put the private variables used in the test inside the test itself, they are only used here.
• Generate a random name for the feature in the test, as is the custom :)
• The variable names for the module directory and the destination directory were a bit ambiguous so I renamed them to $destination and $directory.

These changes do not really impact the patch, so I assume it's OK for me to RTBC this.

Nice work on this one. The commit mentioned in the OP that removed this was a long time ago to combine the fa and fe commands and I'm surprised this didn't get mentioned before.

Seems to be working here, so I committed this to 4efb56d.

This now works with the dev version, but not if you're doing:

drush fe custom/featurename node:page

@nicholasruunu, can you create a new issue for that?

Added hyperlink to commit.