Changes the behavior of goal idea to create a subdirectory named for …

…the project name.

IntelliJ will fail to load the first time if you change the name and try to write the project into
the same directory as an existing project.

Also, enhanced the integration check to examine the contents of the project file when launching "hello"

Testing Done:
Manually confirmed the error "Cannot load module file '/Users/zundel/Development/java/squarepants/idea/foo4568.iml':
File /Users/zundel/Development/java/squarepants/idea/foo4568.iml does not exist
Would you like to remove module 'foo4568' from the project?"

does not occur any more when you change the project name.

Bugs closed: 564, 571

Reviewed at https://rbcommons.com/s/twitter/r/1019/

src/python/pants/backend/jvm/tasks/ide_gen.py

@@ -142,10 +142,15 @@ def __init__(self, *args, **kwargs):
     else:
       self.java_jdk = '1.%d' % self.java_language_level
 
-    self.gen_project_workdir = os.path.abspath(
-      self.context.options.ide_gen_project_dir or
-      os.path.join(self.workdir, self.__class__.__name__, self.project_name)
-    )
+    # Always tack on the project name to the work dir so each project gets its own linked jars,
+    # etc. See https://github.com/pantsbuild/pants/issues/564
+    if self.context.options.ide_gen_project_dir:
+      self.gen_project_workdir = os.path.abspath(
+        os.path.join(self.context.options.ide_gen_project_dir, self.project_name))
+    else:
+      self.gen_project_workdir = os.path.abspath(
+        os.path.join(self.workdir, self.__class__.__name__, self.project_name))
+
     self.cwd = (
       os.path.abspath(self.context.options.ide_gen_project_cwd) if
       self.context.options.ide_gen_project_cwd else self.gen_project_workdir

tests/python/pants_test/tasks/test_eclipse_integration.py

@@ -12,9 +12,10 @@
 
 
 class EclipseIntegrationTest(PantsRunIntegrationTest):
-  def _eclipse_test(self, specs):
+  def _eclipse_test(self, specs, project_dir=os.path.join('.pants.d', 'tmp', 'test-eclipse'),
+      project_name='project'):
     """Helper method that tests eclipse generation on the input spec list."""
-    project_dir = os.path.join('.pants.d', 'tmp', 'test-eclipse')
+
 
     if not os.path.exists(project_dir):
       os.makedirs(project_dir)
@@ -29,12 +30,13 @@ def _eclipse_test(self, specs):
                                                     pants_run.stdout_data))
 
       expected_files = ('.classpath', '.project',)
-      self.assertTrue(os.path.exists(path),
-          'Failed to find project_dir at {dir}.'.format(dir=path))
-      self.assertTrue(all(os.path.exists(os.path.join(path, name))
+      workdir = os.path.join(path, project_name)
+      self.assertTrue(os.path.exists(workdir),
+          'Failed to find project_dir at {dir}.'.format(dir=workdir))
+      self.assertTrue(all(os.path.exists(os.path.join(workdir, name))
           for name in expected_files))
       # return contents of .classpath so we can verify it
-      with open(os.path.join(path, '.classpath')) as classpath_f:
+      with open(os.path.join(workdir, '.classpath')) as classpath_f:
         classpath = classpath_f.read()
       # should be at least one input; if not we may have the wrong target path
       self.assertIn('<classpathentry kind="src"', classpath)

tests/python/pants_test/tasks/test_ensime_integration.py

@@ -13,10 +13,9 @@
 
 class EnsimeIntegrationTest(PantsRunIntegrationTest):
 
-  def _ensime_test(self, specs, project_dir=None):
+  def _ensime_test(self, specs, project_dir = os.path.join('.pants.d', 'tmp-ensime', 'project'),
+      project_name='project'):
     """Helper method that tests ensime generation on the input spec list."""
-    if project_dir is None:
-      project_dir = os.path.join('.pants.d', 'tmp-ensime', 'project')
 
     if not os.path.exists(project_dir):
       os.makedirs(project_dir)
@@ -33,9 +32,10 @@ def _ensime_test(self, specs, project_dir=None):
       # TODO: Actually validate the contents of the project files, rather than just
       # checking if they exist.
       expected_files = ('.ensime',)
-      self.assertTrue(os.path.exists(path),
-          'Failed to find project_dir at {dir}.'.format(dir=path))
-      self.assertTrue(all(os.path.exists(os.path.join(path, name))
+      workdir = os.path.join(path, project_name)
+      self.assertTrue(os.path.exists(workdir),
+          'Failed to find project_dir at {dir}.'.format(dir=workdir))
+      self.assertTrue(all(os.path.exists(os.path.join(workdir, name))
           for name in expected_files), 'Failed to find one of the ensime project files at {dir}'.format(dir=path))
 
   # Testing Ensime integration on a sample project

tests/python/pants_test/tasks/test_idea_integration.py

@@ -8,46 +8,79 @@
 import os
 import xml.dom.minidom as minidom
 
+from pants.base.build_environment import get_buildroot
 from pants.util.contextutil import temporary_dir
 from pants_test.pants_run_integration_test import PantsRunIntegrationTest
 
 
 class IdeaIntegrationTest(PantsRunIntegrationTest):
 
-  def _idea_test(self, specs, project_dir=None, check_func=None):
-    """Helper method that tests idea generation on the input spec list."""
-    if project_dir is None:
-      project_dir = os.path.join('.pants.d', 'tmp', 'test-idea')
+  def _idea_test(self, specs, project_dir=os.path.join('.pants.d', 'idea', 'idea', 'IdeaGen'),
+      project_name=None, check_func=None):
+    """Helper method that tests idea generation on the input spec list.
+    :param project_dir: directory passed to --idea-project-dir
+    :param project_name: name passed to --idea-project-name
+    :param check_func: method to call back with the directory where project files are written.
+    """
 
+    project_dir = os.path.join(get_buildroot(), project_dir)
     if not os.path.exists(project_dir):
       os.makedirs(project_dir)
-    with temporary_dir(root_dir=project_dir) as path:
-      pants_run = self.run_pants(['goal', 'idea',] + specs
-          + ['--no-pantsrc', '--idea-project-dir={dir}'.format(dir=path), '--no-idea-open',
-             '--print-exception-stacktrace', ])
+
+    with temporary_dir(root_dir=project_dir) as project_dir_path:
+
+      extra_flags = ['--idea-project-dir={dir}'.format(dir=project_dir_path)]
+
+      if project_name is None:
+        project_name = "project" # to match Pants' built-in default w/o --idea-project-name
+      else:
+        extra_flags += ['--idea-project-name={name}'.format(name=project_name)]
+
+      all_flags = ['goal', 'idea',] + specs + \
+                  ['--no-pantsrc', '--no-idea-open', '--print-exception-stacktrace' ] + extra_flags
+      pants_run = self.run_pants(all_flags)
       self.assertEquals(pants_run.returncode, self.PANTS_SUCCESS_CODE,
                         "goal idea expected success, got {0}\n"
                         "got stderr:\n{1}\n"
                         "got stdout:\n{2}\n".format(pants_run.returncode,
                                                     pants_run.stderr_data,
                                                     pants_run.stdout_data))
-      # TODO(Garrett Malmquist): Actually validate the contents of the project files, rather than just
-      # checking if they exist.
-      expected_files = ('project.iml', 'project.ipr',)
-      self.assertTrue(os.path.exists(path),
-          'Failed to find project_dir at {dir}.'.format(dir=path))
-      self.assertTrue(all(os.path.exists(os.path.join(path, name))
-          for name in expected_files))
+
+      expected_files = ('{project_name}.iml'.format(project_name=project_name),
+                        '{project_name}.ipr'.format(project_name=project_name))
+
+      workdir = os.path.join(project_dir_path, project_name)
+      self.assertTrue(os.path.exists(workdir),
+                      'exec ./pants {all_flags}. Failed to find project_dir at {dir}.'
+                      .format(all_flags=" ".join(all_flags), dir=workdir))
+      self.assertTrue(all(os.path.exists(os.path.join(workdir, name))
+                          for name in expected_files),
+                      msg="Failed to exec ./pants {all_flags}".format(all_flags=all_flags))
       if check_func:
-        check_func(path)
+        check_func(workdir)
+
+  def _get_sourceFolders(self, dom):
+    """Navigate the dom to return the list of all <sourceFolder> entries in the project file"""
+    module = dom.getElementsByTagName('module')[0]
+    components = module.getElementsByTagName('component')
+    for component in components:
+      if component.getAttribute('name') == 'NewModuleRootManager':
+        content = module.getElementsByTagName('content')[0]
+        return content.getElementsByTagName('sourceFolder')
+    return []
 
   # Testing IDEA integration on lots of different targets which require different functionalities to
   # make sure that everything that needs to happen for idea gen does happen.
-
+  # TODO(Garrett Malmquist): Actually validate the contents of the project files, rather than just
+  # checking if they exist.
   def test_idea_on_alternate_project_dir(self):
     alt_dir = os.path.join('.pants.d', 'tmp', 'some', 'random', 'directory', 'for', 'idea', 'stuff')
     self._idea_test(['examples/src/java/com/pants/examples/hello::'], project_dir=alt_dir)
 
+  def test_idea_alternate_name(self):
+    alt_name = "alt-name"
+    self._idea_test(['examples/src/java/com/pants/examples/hello::'], project_name=alt_name)
+
   def test_idea_on_protobuf(self):
     self._idea_test(['examples/src/java/com/pants/examples/protobuf::'])
 
@@ -58,7 +91,30 @@ def test_idea_on_unicode(self):
     self._idea_test(['testprojects/src/java/com/pants/testproject/unicode::'])
 
   def test_idea_on_hello(self):
-    self._idea_test(['examples/src/java/com/pants/examples/hello::'])
+    def do_check(path):
+      """Check to see that the project contains the expected source folders."""
+      foundSourceContent = False
+      iml_file = os.path.join(path, 'project.iml')
+      self.assertTrue(os.path.exists(iml_file))
+      dom = minidom.parse(iml_file)
+      expected_paths = ["file://" + os.path.join(get_buildroot(), path) for path in [
+        'examples/src/java/com/pants/example/hello',
+        'examples/src/java/com/pants/examples/hello/greet',
+        'examples/src/java/com/pants/examples/hello/main',
+        'examples/src/resources/com/pants/example/hello',
+      ]]
+      remaining = set(expected_paths)
+      for sourceFolder in self._get_sourceFolders(dom):
+        foundSourceContent = True
+        self.assertEquals("False", sourceFolder.getAttribute('isTestSource'))
+        url = sourceFolder.getAttribute('url')
+        self.assertIn(url, remaining,
+                       msg="Couldn't find url={url} in {expected}".format(url=url,
+                                                                          expected=expected_paths))
+        remaining.remove(url)
+      self.assertTrue(foundSourceContent)
+
+    self._idea_test(['examples/src/java/com/pants/examples/hello::'], check_func=do_check)
 
   def test_idea_on_annotations(self):
     self._idea_test(['examples/src/java/com/pants/examples/annotation::'])
@@ -78,39 +134,36 @@ def test_idea_on_scaladepsonboth(self):
   def test_idea_on_maven_layout(self):
     def do_check(path):
       """
-          The contents of the .iml file should have sourceFolder entries that look like:
+          The contents of the .iml file should have sourceFolder entries that all look like:
           <sourceFolder url=".../src/main/java"  isTestSource="False"/>
           <sourceFolder url=".../src/main/resources"  isTestSource="False"/>
           <sourceFolder url=".../src/test/java"  isTestSource="True"/>
           <sourceFolder url=".../src/test/resources"  isTestSource="True"/>
           ...
       """
       foundSourceContent = False
-      dom = minidom.parse(os.path.join(path, 'project.iml'))
-      module = dom.getElementsByTagName('module')[0]
-      components = module.getElementsByTagName('component')
-
-      for component in components:
-        if component.getAttribute('name') == 'NewModuleRootManager':
-          content = module.getElementsByTagName('content')[0]
-          sourceFolders = content.getElementsByTagName('sourceFolder')
-          for sourceFolder in sourceFolders:
-            foundSourceContent = True
-            url = sourceFolder.getAttribute('url')
-            is_test_source = sourceFolder.getAttribute('isTestSource')
-            if url.endswith("src/main/java") or url.endswith("src/main/resources"):
-              self.assertEquals("False", is_test_source,
-                               msg="wrong test flag: url={url} isTestSource={is_test_source}"
-                               .format(url=url, is_test_source=is_test_source))
-            elif url.endswith("src/test/java") or url.endswith("src/test/resources"):
-              self.assertEquals("True", is_test_source,
-                              msg="wrong test flag: url={url} isTestSource={is_test_source}"
-                              .format(url=url, is_test_source=is_test_source))
-            else:
-              self.fail("Unexpected sourceContent tag: url={url} isTestSource={is_test_source}"
-              .format(url=url, is_test_source=is_test_source))
+      iml_file = os.path.join(path, 'project.iml')
+      self.assertTrue(os.path.exists(iml_file))
+      dom = minidom.parse(iml_file)
+      for sourceFolder in  self._get_sourceFolders(dom):
+        foundSourceContent = True
+        url = sourceFolder.getAttribute('url')
+        is_test_source = sourceFolder.getAttribute('isTestSource')
+        if url.endswith("src/main/java") or url.endswith("src/main/resources"):
+          self.assertEquals("False", is_test_source,
+                           msg="wrong test flag: url={url} isTestSource={is_test_source}"
+                           .format(url=url, is_test_source=is_test_source))
+        elif url.endswith("src/test/java") or url.endswith("src/test/resources"):
+          self.assertEquals("True", is_test_source,
+                          msg="wrong test flag: url={url} isTestSource={is_test_source}"
+                          .format(url=url, is_test_source=is_test_source))
+        else:
+          self.fail("Unexpected sourceContent tag: url={url} isTestSource={is_test_source}"
+          .format(url=url, is_test_source=is_test_source))
       self.assertTrue(foundSourceContent)
 
     self._idea_test(['testprojects/maven_layout/resource_collision::', '--idea-use-source-root',
                      '--idea-infer-test-from-siblings',],
                     check_func=do_check)
+
+