Restore ChangedTargetGoalsIntegrationTest.

All of its tests were marked xfail but are now re-enabled with changes
to ensure the targets and git repo under test are isolated from the
host pants repo.

In order to facilitate integration tests using an ephemeral git repo,
a change is made to `build-support/pants_venv` to not rely on git to
find the host pants repo root directory.

Testing Done:
Locally green: `./pants test-changed`.

CI went green here:
  https://travis-ci.org/pantsbuild/pants/builds/159416022

Bugs closed: 3859

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

build-support/common.sh

@@ -1,7 +1,5 @@
 #!/usr/bin/env bash
 
-
-
 function log() {
   echo -e "$@" 1>&2
 }

build-support/pants_venv

@@ -2,7 +2,7 @@
 # Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-REPO_ROOT=$(cd $(dirname "${BASH_SOURCE[0]}") && cd "$(git rev-parse --show-toplevel)" && pwd)
+REPO_ROOT=$(cd $(dirname "${BASH_SOURCE[0]}") && cd .. && pwd -P)
 source ${REPO_ROOT}/build-support/common.sh
 
 REQUIREMENTS=(

tests/python/pants_test/tasks/BUILD

@@ -26,8 +26,11 @@ python_tests(
   name = 'changed_target_integration',
   sources = ['test_changed_target_integration.py'],
   dependencies = [
+    'src/python/pants/base:build_environment',
     'src/python/pants/util:contextutil',
+    'src/python/pants/util:dirutil',
     'tests/python/pants_test:int-test',
+    'tests/python/pants_test/testutils:git_util',
   ],
   tags = {'integration'},
 )

tests/python/pants_test/tasks/test_changed_target_integration.py

@@ -6,56 +6,123 @@
                         unicode_literals, with_statement)
 
 import os
-from unittest import expectedFailure
+from contextlib import contextmanager
+from textwrap import dedent
 
+from pants.base.build_environment import get_buildroot
+from pants.util.contextutil import temporary_dir
+from pants.util.dirutil import safe_open
 from pants_test.pants_run_integration_test import PantsRunIntegrationTest
+from pants_test.testutils.git_util import initialize_repo
 
 
 class ChangedTargetGoalsIntegrationTest(PantsRunIntegrationTest):
-  def ref_for_greet_change(self):
-    # Unfortunately, as in being an integration test, it is difficult to mock the SCM used.
-    # Thus this will use the pants commit log, so we need a commit that changes greet example.
-    # Any comment/whitespace/etc change is enough though, as long as we know the SHA.
-    return '14cc5bc23561918dc7134427bfcb268506fcbcaa'
+  @contextmanager
+  def known_commits(self):
+    with temporary_dir(root_dir=get_buildroot()) as worktree:
+      with safe_open(os.path.join(worktree, 'README'), 'w') as fp:
+        fp.write('Just a test tree.')
+
+      with initialize_repo(worktree=worktree, gitdir=os.path.join(worktree, '.git')) as git:
+        src_file = os.path.join(worktree, 'src/java/org/pantsbuild/Class.java')
+        with safe_open(src_file, 'w') as fp:
+          fp.write(dedent("""
+          package org.pantsbuild;
+
+          class Class {
+            static final int MEANING_OF_LIFE = 42;
+          }
+          """))
+
+        src_build_file = os.path.join(worktree, 'src/java/org/pantsbuild/BUILD')
+        with safe_open(src_build_file, 'w') as fp:
+          fp.write("java_library(name='pantsbuild', sources=['Class.java'])")
+
+        git.add(src_file, src_build_file)
+        git.commit('Introduce Class.')
+
+        test_file = os.path.join(worktree, 'tests/java/org/pantsbuild/ClassTest.java')
+        with safe_open(test_file, 'w') as fp:
+          fp.write(dedent("""
+          package org.pantsbuild;
+
+          import org.junit.Assert;
+          import org.junit.Test;
+
+          public class ClassTest {
+            @Test public void test() {
+              Assert.assertEquals(42, Class.MEANING_OF_LIFE);
+            }
+          }
+          """))
+
+        test_build_file = os.path.join(worktree, 'tests/java/org/pantsbuild/BUILD')
+        with safe_open(test_build_file, 'w') as fp:
+          fp.write(dedent("""
+          jar_library(name='junit', jars=[jar('junit', 'junit', '4.12')])
+
+          junit_tests(
+            name='pantsbuild',
+            sources=['ClassTest.java'],
+            dependencies=[
+              ':junit',
+              '{}'
+            ]
+          )
+          """).format(os.path.relpath(os.path.dirname(src_build_file), get_buildroot())))
+
+        git.add(test_file, test_build_file)
+        git.commit('Introduce ClassTest.')
+
+        yield
+
+  _PACKAGE_PATH_PREFIX = os.sep + os.path.join('classes', 'org', 'pantsbuild')
+
+  def find_classfile(self, workdir, filename):
+    for root, dirs, files in os.walk(os.path.join(workdir, 'compile', 'zinc')):
+      for f in files:
+        candidate = os.path.join(root, f)
+        if candidate.endswith(os.path.join(self._PACKAGE_PATH_PREFIX, filename)):
+          return candidate
 
-  def greet_classfile(self, workdir, filename):
-    path = 'compile/jvm/java/classes/org/pantsbuild/example/hello/greet'.split('/')
-    return os.path.join(workdir, *(path + [filename]))
-
-  @expectedFailure
   def test_compile_changed(self):
-    cmd = ['compile-changed', '--diffspec={}'.format(self.ref_for_greet_change())]
-
-    with self.temporary_workdir() as workdir:
-      # Nothing exists.
-      self.assertFalse(os.path.exists(self.greet_classfile(workdir, 'Greeting.class')))
-      self.assertFalse(os.path.exists(self.greet_classfile(workdir, 'GreetingTest.class')))
-
-      run = self.run_pants_with_workdir(cmd, workdir)
-      self.assert_success(run)
-
-      # The directly changed target's produced classfile exists.
-      self.assertTrue(os.path.exists(self.greet_classfile(workdir, 'Greeting.class')))
-      self.assertFalse(os.path.exists(self.greet_classfile(workdir, 'GreetingTest.class')))
-
-    with self.temporary_workdir() as workdir:
-      # Nothing exists.
-      self.assertFalse(os.path.exists(self.greet_classfile(workdir, 'Greeting.class')))
-      self.assertFalse(os.path.exists(self.greet_classfile(workdir, 'GreetingTest.class')))
-
-      run = self.run_pants_with_workdir(cmd + ['--include-dependees=direct'], workdir)
-      self.assert_success(run)
-
-      # The changed target's and its direct dependees' (eg its tests) classfiles exist.
-      self.assertTrue(os.path.exists(self.greet_classfile(workdir, 'Greeting.class')))
-      self.assertTrue(os.path.exists(self.greet_classfile(workdir, 'GreetingTest.class')))
+    with self.known_commits():
+      # Just look for changes in the 1st commit (addition of Class.java).
+      cmd = ['compile-changed', '--diffspec=HEAD~2..HEAD~1']
+
+      with self.temporary_workdir() as workdir:
+        # Nothing exists.
+        self.assertIsNone(self.find_classfile(workdir, 'Class.class'))
+        self.assertIsNone(self.find_classfile(workdir, 'ClassTest.class'))
+
+        run = self.run_pants_with_workdir(cmd, workdir)
+        self.assert_success(run)
+
+        # The directly changed target's produced classfile exists.
+        self.assertIsNotNone(self.find_classfile(workdir, 'Class.class'))
+        self.assertIsNone(self.find_classfile(workdir, 'ClassTest.class'))
+
+      with self.temporary_workdir() as workdir:
+        # Nothing exists.
+        self.assertIsNone(self.find_classfile(workdir, 'Class.class'))
+        self.assertIsNone(self.find_classfile(workdir, 'ClassTest.class'))
+
+        run = self.run_pants_with_workdir(cmd + ['--include-dependees=direct'], workdir)
+        self.assert_success(run)
+
+        # The changed target's and its direct dependees' (eg its tests) classfiles exist.
+        # NB: This highlights a quirk of test-changed (really ChangeCalculator): it uses a
+        # potentially historical diff to calculate changed files but it always uses BUILD files
+        # from HEAD to determine dependees.  As such, in this case, although neither ClassTest.java
+        # nor its BUILD file existed in HEAD~2...HEAD~1 - they do now and so are seen as dependees
+        # of Class.java.
+        self.assertIsNotNone(self.find_classfile(workdir, 'Class.class'))
+        self.assertIsNotNone(self.find_classfile(workdir, 'ClassTest.class'))
 
-  @expectedFailure
   def test_test_changed(self):
-    with self.temporary_workdir() as workdir:
-      cmd = ['test-changed', '--diffspec={}'.format(self.ref_for_greet_change())]
-      junit_out = os.path.join(workdir, 'test', 'junit',
-        'org.pantsbuild.example.hello.greet.GreetingTest.out.txt')
+    with self.known_commits(), self.temporary_workdir() as workdir:
+      cmd = ['test-changed', '--diffspec=HEAD~2..HEAD~1']
+      junit_out = os.path.join(workdir, 'test', 'junit', 'org.pantsbuild.ClassTest.out.txt')
 
       self.assertFalse(os.path.exists(junit_out))
 

tests/python/pants_test/testutils/BUILD

@@ -24,5 +24,6 @@ python_library(
   dependencies = [
     'src/python/pants/base:revision',
     'src/python/pants/scm:git',
+    'src/python/pants/util:contextutil',
   ],
 )

tests/python/pants_test/testutils/git_util.py

@@ -11,8 +11,7 @@
 
 from pants.base.revision import Revision
 from pants.scm.git import Git
-from pants.util.contextutil import environment_as
-from pants.util.dirutil import safe_mkdtemp, safe_rmtree
+from pants.util.contextutil import environment_as, temporary_dir
 
 
 MIN_REQUIRED_GIT_VERSION = Revision.semver('1.7.10')
@@ -29,16 +28,30 @@ def git_version():
 
 
 @contextmanager
-def initialize_repo(worktree):
-  """Initialize git repository for the given worktree."""
-  gitdir = safe_mkdtemp()
-  with environment_as(GIT_DIR=gitdir, GIT_WORK_TREE=worktree):
+def initialize_repo(worktree, gitdir=None):
+  """Initialize a git repository for the given `worktree`.
+
+  NB: The given `worktree` must contain at least one file which will be committed to form an initial
+  commit.
+
+  :param string worktree: The path to the git work tree.
+  :param string gitdir: An optional path to the `.git` dir to use.
+  :returns: A `Git` repository object that can be used to interact with the repo.
+  :rtype: :class:`pants.scm.git.Git`
+  """
+  @contextmanager
+  def use_gitdir():
+    if gitdir:
+      yield gitdir
+    else:
+      with temporary_dir() as d:
+        yield d
+
+  with use_gitdir() as git_dir, environment_as(GIT_DIR=git_dir, GIT_WORK_TREE=worktree):
     subprocess.check_call(['git', 'init'])
     subprocess.check_call(['git', 'config', 'user.email', '[email protected]'])
     subprocess.check_call(['git', 'config', 'user.name', 'Your Name'])
     subprocess.check_call(['git', 'add', '.'])
     subprocess.check_call(['git', 'commit', '-am', 'Add project files.'])
 
-    yield Git(gitdir=gitdir, worktree=worktree)
-
-    safe_rmtree(gitdir)
+    yield Git(gitdir=git_dir, worktree=worktree)