migrate changed integration tests to isolated temp git repos and add …

…an environment variable to override buildroot Currently, the integration tests in tests/python/pants_test/engine/legacy/test_changed_integration.py rely on mutating the working copy for the purposes of examining real scm diff information to support the changed suite of goals. this means that if there are uncommitted changes, the git diff output will be altered which can subsequently cause the integration tests to fail. This change does: 1. Each changed integration test creates an isolated git repo and runs inside it. Note: there are 4 tests that do not mutate working copy, thus they are exempted. The isolated git repo is a subdir of buildroot. The current logic will cause git to panic in this case. Thus I set buildroot to the same git dir as well. 2. Modify get_buildroot(). It will first look for an environment variable PANTS_BUILDROOT_OVERRIDE. If set, then its value will be the buildroot path. If not, the usual way of determining buildroot is used. This env var is purely for testing usage. 3. Shuffle targets in pants_test/base/BUILD to follow alphabetical order of target names. Testing Done: https://travis-ci.org/pantsbuild/pants/builds/167397682 Bugs closed: 3906, 3946 Reviewed at https://rbcommons.com/s/twitter/r/4295/
beejiujitsu · Oct 13, 2016 · 8fe2479 · 8fe2479
1 parent 3f3abd9
commit 8fe2479
Show file tree

Hide file tree

Showing 6 changed files with 325 additions and 182 deletions.
diff --git a/src/python/pants/base/build_root.py b/src/python/pants/base/build_root.py
@@ -48,7 +48,12 @@ def __init__(self):
   def path(self):
     """Returns the build root for the current workspace."""
     if self._root_dir is None:
-      self._root_dir = os.path.realpath(self.find_buildroot())
+      # This env variable is for testing purpose.
+      override_buildroot = os.environ.get('PANTS_BUILDROOT_OVERRIDE', None)
+      if override_buildroot:
+        self._root_dir = override_buildroot
+      else:
+        self._root_dir = os.path.realpath(self.find_buildroot())
     return self._root_dir
 
   @path.setter

diff --git a/tests/python/pants_test/base/BUILD b/tests/python/pants_test/base/BUILD
@@ -1,18 +1,6 @@
 # Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-python_library(
-  name = 'context_utils',
-  sources = ['context_utils.py'],
-  dependencies = [
-    '3rdparty/python/twitter/commons:twitter.common.collections',
-    'src/python/pants/base:workunit',
-    'src/python/pants/build_graph',
-    'src/python/pants/goal:context',
-    'tests/python/pants_test/option/util',
-  ]
-)
-
 python_tests(
   name = 'build_environment',
   sources = ['test_build_environment.py'],
@@ -34,50 +22,46 @@ python_library(
 )
 
 python_tests(
-  name = 'filesystem_build_file',
-  sources = ['test_filesystem_build_file.py'],
-  dependencies = [
-    ':build_file_test_base',
-    '3rdparty/python/twitter/commons:twitter.common.collections',
-    '3rdparty/python:six',
-    'src/python/pants/base:build_file',
-    'src/python/pants/util:dirutil',
-  ]
-)
-
-python_library(
-  name = 'project_tree_test_base',
-  sources = ['project_tree_test_base.py'],
+  name = 'build_root',
+  sources = ['test_build_root.py'],
   dependencies = [
+    'src/python/pants/base:build_root',
+    'src/python/pants/util:contextutil',
     'src/python/pants/util:dirutil',
-    'src/python/pants/util:meta',
+    'tests/python/pants_test:base_test',
   ]
 )
 
-python_library(
-  name = 'pants_ignore_test_base',
-  sources = ['pants_ignore_test_base.py'],
+python_tests(
+  name = 'bundle_integration',
+  sources = [ 'test_bundle_integration.py' ],
   dependencies = [
-    ':project_tree_test_base',
-  ]
+    'tests/python/pants_test:int-test',
+  ],
+  tags = {'integration'},
+  timeout = 120,
 )
 
 python_tests(
-  name = 'pants_ignore_file_system',
-  sources = ['test_pants_ignore_file_system.py'],
+  name = 'cmd_line_spec_parser',
+  sources = ['test_cmd_line_spec_parser.py'],
   dependencies = [
-    ':pants_ignore_test_base',
-    'src/python/pants/base:project_tree',
+    '3rdparty/python/twitter/commons:twitter.common.collections',
+    'src/python/pants/base:cmd_line_spec_parser',
+    'src/python/pants/build_graph',
+    'tests/python/pants_test:base_test',
   ]
 )
 
-python_tests(
-  name = 'pants_ignore_scm',
-  sources = ['test_pants_ignore_scm.py'],
+python_library(
+  name = 'context_utils',
+  sources = ['context_utils.py'],
   dependencies = [
-    ':pants_ignore_test_base',
-    'src/python/pants/base:project_tree',
-    'src/python/pants/scm:git',
+    '3rdparty/python/twitter/commons:twitter.common.collections',
+    'src/python/pants/base:workunit',
+    'src/python/pants/build_graph',
+    'src/python/pants/goal:context',
+    'tests/python/pants_test/option/util',
   ]
 )
 
@@ -91,24 +75,14 @@ python_tests(
 )
 
 python_tests(
-  name = 'build_root',
-  sources = ['test_build_root.py'],
-  dependencies = [
-    'src/python/pants/base:build_root',
-    'tests/python/pants_test:base_test',
-    'src/python/pants/util:contextutil',
-    'src/python/pants/util:dirutil',
-  ]
-)
-
-python_tests(
-  name = 'cmd_line_spec_parser',
-  sources = ['test_cmd_line_spec_parser.py'],
+  name = 'filesystem_build_file',
+  sources = ['test_filesystem_build_file.py'],
   dependencies = [
+    ':build_file_test_base',
+    '3rdparty/python:six',
     '3rdparty/python/twitter/commons:twitter.common.collections',
-    'src/python/pants/base:cmd_line_spec_parser',
-    'src/python/pants/build_graph',
-    'tests/python/pants_test:base_test',
+    'src/python/pants/base:build_file',
+    'src/python/pants/util:dirutil',
   ]
 )
 
@@ -139,6 +113,33 @@ python_tests(
   ]
 )
 
+python_tests(
+  name = 'pants_ignore_file_system',
+  sources = ['test_pants_ignore_file_system.py'],
+  dependencies = [
+    ':pants_ignore_test_base',
+    'src/python/pants/base:project_tree',
+  ]
+)
+
+python_tests(
+  name = 'pants_ignore_scm',
+  sources = ['test_pants_ignore_scm.py'],
+  dependencies = [
+    ':pants_ignore_test_base',
+    'src/python/pants/base:project_tree',
+    'src/python/pants/scm:git',
+  ]
+)
+
+python_library(
+  name = 'pants_ignore_test_base',
+  sources = ['pants_ignore_test_base.py'],
+  dependencies = [
+    ':project_tree_test_base',
+  ]
+)
+
 python_tests(
   name = 'payload',
   sources = ['test_payload.py'],
@@ -161,6 +162,15 @@ python_tests(
   ]
 )
 
+python_library(
+  name = 'project_tree_test_base',
+  sources = ['project_tree_test_base.py'],
+  dependencies = [
+    'src/python/pants/util:dirutil',
+    'src/python/pants/util:meta',
+  ]
+)
+
 python_tests(
   name = 'revision',
   sources = ['test_revision.py'],
@@ -192,16 +202,6 @@ python_tests(
   ]
 )
 
-python_tests(
-  name = 'bundle_integration',
-  sources = [ 'test_bundle_integration.py' ],
-  dependencies = [
-    'tests/python/pants_test:int-test',
-  ],
-  tags = {'integration'},
-  timeout = 120,
-)
-
 python_tests(
   name = 'worker_pool',
   sources = ['test_worker_pool.py'],

diff --git a/tests/python/pants_test/base/test_build_root.py b/tests/python/pants_test/base/test_build_root.py
@@ -9,7 +9,7 @@
 import unittest
 
 from pants.base.build_root import BuildRoot
-from pants.util.contextutil import pushd, temporary_dir
+from pants.util.contextutil import environment_as, pushd, temporary_dir
 from pants.util.dirutil import safe_mkdir, safe_mkdtemp, safe_rmtree, touch
 
 
@@ -61,3 +61,8 @@ def test_not_found(self):
       root = os.path.realpath(root)
       with pushd(root):
         self.assertRaises(BuildRoot.NotFoundError, lambda: BuildRoot().path)
+
+  def test_buildroot_override(self):
+    with temporary_dir() as root:
+      with environment_as(PANTS_BUILDROOT_OVERRIDE=root):
+        self.assertEqual(BuildRoot().path, root)
diff --git a/tests/python/pants_test/engine/legacy/BUILD b/tests/python/pants_test/engine/legacy/BUILD
@@ -33,6 +33,21 @@ python_tests(
   tags = {'integration'}
 )
 
+python_tests(
+  name = 'changed_integration',
+  sources = ['test_changed_integration.py'],
+  dependencies = [
+    'src/python/pants/base:build_environment',
+    'src/python/pants/util:contextutil',
+    'src/python/pants/util:dirutil',
+    'tests/python/pants_test:base_test',
+    'tests/python/pants_test:int-test',
+    'tests/python/pants_test/testutils:git_util',
+  ],
+  tags = {'integration'},
+  timeout = 600,
+)
+
 python_tests(
   name = 'dependees_integration',
   sources = ['test_dependees_integration.py'],
@@ -73,14 +88,6 @@ python_tests(
   ]
 )
 
-python_tests(
-  name = 'structs',
-  sources = ['test_structs.py'],
-  dependencies = [
-    'src/python/pants/engine/legacy:structs',
-  ]
-)
-
 python_tests(
   name = 'list_integration',
   sources = ['test_list_integration.py'],
@@ -101,12 +108,9 @@ python_tests(
 )
 
 python_tests(
-  name = 'changed_integration',
-  sources = ['test_changed_integration.py'],
+  name = 'structs',
+  sources = ['test_structs.py'],
   dependencies = [
-    'tests/python/pants_test:base_test',
-    'tests/python/pants_test:int-test'
-  ],
-  tags = {'integration'},
-  timeout = 600,
-)
+    'src/python/pants/engine/legacy:structs',
+  ]
+)