From b7c83046c4dca4631874501266e443f28b2cb120 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Sun, 29 Sep 2019 15:57:56 -0700 Subject: [PATCH 1/7] Only get specific target types --- build-support/bin/ci.py | 2 +- .../projects/test_testprojects_integration.py | 91 +++++++++++++++---- 2 files changed, 72 insertions(+), 21 deletions(-) diff --git a/build-support/bin/ci.py b/build-support/bin/ci.py index 2d7c410a3e3..bd0358792e9 100755 --- a/build-support/bin/ci.py +++ b/build-support/bin/ci.py @@ -257,8 +257,8 @@ def get_listed_targets(filename: str) -> TargetSet: [ "./pants.pex", f"--tag={'-' if test_type == TestType.unit else '+'}integration", - "--filter-type=python_tests", "filter", + "--type=python_tests", "src/python::", "tests/python::", "contrib::", diff --git a/tests/python/pants_test/projects/test_testprojects_integration.py b/tests/python/pants_test/projects/test_testprojects_integration.py index 7354469cd0f..f4fd283291c 100644 --- a/tests/python/pants_test/projects/test_testprojects_integration.py +++ b/tests/python/pants_test/projects/test_testprojects_integration.py @@ -2,6 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import math +from typing import List, Set from pants.util.memo import memoized_property from pants_test.pants_run_integration_test import PantsRunIntegrationTest @@ -14,10 +15,8 @@ class TestProjectsIntegrationTest(PantsRunIntegrationTest, AbstractTestGenerator # on), we shard all of the targets under `testprojects` into _SHARDS test methods. _SHARDS = 256 - @memoized_property - def targets(self): - """A sequence of target name strings.""" - + @property + def skipped_targets(self) -> List[str]: # Targets that fail but shouldn't known_failing_targets = [ # The following two targets lose out due to a resource collision, because `example_b` happens @@ -84,7 +83,7 @@ def targets(self): # Interpreter will not resolve correctly when Pants is constrained to Python 3 python2_only = [ # tested in test_antlr_py_gen_integration.py - 'testprojects/src/python/antlr' + 'testprojects/src/python/antlr', ] # Targets for testing timeouts. These should only be run during specific integration tests, @@ -102,47 +101,99 @@ def targets(self): ] simply_skip = [ - # Already tested at pants_test.backend.jvm.targets.test_jar_dependency_integration.JarDependencyIntegrationTest + # Already tested at + # pants_test.backend.jvm.targets.test_jar_dependency_integration.JarDependencyIntegrationTest 'testprojects/3rdparty/org/pantsbuild/testprojects:testprojects', # Already tested in 'PantsRequirementIntegrationTest' and 'SetupPyIntegrationTest'. 'testprojects/pants-plugins/*', 'testprojects/src/python/python_distribution/ctypes:ctypes_test', 'testprojects/src/python/python_distribution/ctypes_with_third_party:ctypes_test', 'testprojects/src/python/python_distribution/ctypes_with_extra_compiler_flags:bin', - # Requires non-standard settings, and already tested by `ProtobufIntegrationTest.test_import_from_buildroot` + # Requires non-standard settings, and already tested by + # `ProtobufIntegrationTest.test_import_from_buildroot` 'testprojects/src/protobuf/org/pantsbuild/testproject/import_from_buildroot.*', ] - targets_to_exclude = (known_failing_targets + negative_test_targets + need_java_8 + python2_only + - timeout_targets + deliberately_conflicting_targets + simply_skip) - exclude_opts = ['--exclude-target-regexp={}'.format(target) for target in targets_to_exclude] + return [ + *known_failing_targets, + *negative_test_targets, + *need_java_8, + *python2_only, + *timeout_targets, + *deliberately_conflicting_targets, + *simply_skip, + ] - # Run list with exclude options, then parse and sort output. - pants_run = self.run_pants(['list', 'testprojects::', 'examples::'] + exclude_opts) - self.assert_success(pants_run) - return sorted(pants_run.stdout_data.split()) + @memoized_property + def targets(self) -> List[str]: + """A sequence of target name strings.""" + + exclude_opts = [f'--exclude-target-regexp={target}' for target in self.skipped_targets] + + def get_targets_for_type(build_file_alias: str) -> Set[str]: + pants_run = self.run_pants([ + 'filter', + f'--type={build_file_alias}', + 'testprojects::', + 'examples::', + *exclude_opts + ]) + self.assert_success(pants_run) + return set(pants_run.stdout_data.split()) + + targets = set() + # TODO: map these build_file_aliases to more targeted goals than `test`. Right now, we use + # `test` to trigger side effects via the goal graph, such as compiling JVM code or generating + # antlr code. Instead, we should run the specific command we are testing for that target type. + # For example, `python_antlr_library` would only need `./pants gen`. + for bfa in [ + # test targets + "python_tests", + "junit_tests", + # library targets + "python_library", + "java_library", + "jar_library", + "scala_library", + # binary targets + "python_binary", + "jvm_binary", + # bundle targets + "jvm_app", + # codegen targets + "python_antlr_library", + "python_thrift_library", + "java_antlr_library", + "java_protobuf_library", + "java_thrift_library", + "java_wire_library", + ]: + targets.update(get_targets_for_type(bfa)) + return list(sorted(targets)) def targets_for_shard(self, shard): if shard < 0 or shard >= self._SHARDS: - raise Exception('Invalid shard: {} / {}'.format(shard, self._SHARDS)) + raise Exception(f'Invalid shard: {shard} / {self._SHARDS}') per_shard = int(math.ceil(len(self.targets) / self._SHARDS)) - offset = (per_shard * shard) + offset = per_shard * shard return self.targets[offset:offset + per_shard] def run_shard(self, shard): targets = self.targets_for_shard(shard) - pants_run = self.run_pants(['test'] + targets + ['--jvm-platform-default-platform=java7']) + pants_run = self.run_pants(['test', *targets, '--jvm-platform-default-platform=java7']) self.assert_success(pants_run) def test_self(self): - self.assertEqual([t for s in range(0, self._SHARDS) - for t in self.targets_for_shard(s)], - self.targets) + self.assertEqual( + [t for s in range(0, self._SHARDS) for t in self.targets_for_shard(s)], + self.targets + ) @classmethod def generate_tests(cls): for shardid in range(0, cls._SHARDS): cls.add_test(f'test_shard_{shardid}', lambda this: this.run_shard(shardid)) + TestProjectsIntegrationTest.generate_tests() From d9bf7ba2a974a5fa26d315868bfe6fd7bfe8d251 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Fri, 4 Oct 2019 17:18:48 -0700 Subject: [PATCH 2/7] Remove jar_library, which == 3rd party Jar --- .../python/pants_test/projects/test_testprojects_integration.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/python/pants_test/projects/test_testprojects_integration.py b/tests/python/pants_test/projects/test_testprojects_integration.py index f4fd283291c..1f587fe4f77 100644 --- a/tests/python/pants_test/projects/test_testprojects_integration.py +++ b/tests/python/pants_test/projects/test_testprojects_integration.py @@ -153,7 +153,6 @@ def get_targets_for_type(build_file_alias: str) -> Set[str]: # library targets "python_library", "java_library", - "jar_library", "scala_library", # binary targets "python_binary", From f6231d34f8d8dd7f8a054ec136c50dbb174465ae Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Fri, 4 Oct 2019 17:21:20 -0700 Subject: [PATCH 3/7] Bump timeout --- tests/python/pants_test/projects/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/python/pants_test/projects/BUILD b/tests/python/pants_test/projects/BUILD index 0d773cf99d5..6619b65ce37 100644 --- a/tests/python/pants_test/projects/BUILD +++ b/tests/python/pants_test/projects/BUILD @@ -10,5 +10,5 @@ python_tests( 'tests/python/pants_test:test_base', ], tags = {'integration'}, - timeout=950, + timeout=1400, ) From 854f62b0a9498e69c9ac98d012ce6e77c7a7160a Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Sat, 5 Oct 2019 08:46:25 -0700 Subject: [PATCH 4/7] New approach: 1) Blacklist certain target types, rather than whitelisting 2) Abandon idea of specific commands based on the target type. Everyone uses `./pants compile lint test` now --- .../projects/test_testprojects_integration.py | 71 ++++++++----------- 1 file changed, 30 insertions(+), 41 deletions(-) diff --git a/tests/python/pants_test/projects/test_testprojects_integration.py b/tests/python/pants_test/projects/test_testprojects_integration.py index 1f587fe4f77..07587ad25a3 100644 --- a/tests/python/pants_test/projects/test_testprojects_integration.py +++ b/tests/python/pants_test/projects/test_testprojects_integration.py @@ -124,51 +124,38 @@ def skipped_targets(self) -> List[str]: *simply_skip, ] + @property + def skipped_target_types(self) -> List[str]: + """We don't want to run over every single target, e.g. files() are only used for us to depend + on in ITs and it doesn't make sense to run `./pants test` against them.""" + return [ + "target", + # resources / loose files + "files", + "resources", + "page", + # 3rd-party dependencies + "jar_library", + "managed_jar_libraries", + "python_requirement_library", + ] + @memoized_property def targets(self) -> List[str]: """A sequence of target name strings.""" exclude_opts = [f'--exclude-target-regexp={target}' for target in self.skipped_targets] - - def get_targets_for_type(build_file_alias: str) -> Set[str]: - pants_run = self.run_pants([ - 'filter', - f'--type={build_file_alias}', - 'testprojects::', - 'examples::', - *exclude_opts - ]) - self.assert_success(pants_run) - return set(pants_run.stdout_data.split()) - - targets = set() - # TODO: map these build_file_aliases to more targeted goals than `test`. Right now, we use - # `test` to trigger side effects via the goal graph, such as compiling JVM code or generating - # antlr code. Instead, we should run the specific command we are testing for that target type. - # For example, `python_antlr_library` would only need `./pants gen`. - for bfa in [ - # test targets - "python_tests", - "junit_tests", - # library targets - "python_library", - "java_library", - "scala_library", - # binary targets - "python_binary", - "jvm_binary", - # bundle targets - "jvm_app", - # codegen targets - "python_antlr_library", - "python_thrift_library", - "java_antlr_library", - "java_protobuf_library", - "java_thrift_library", - "java_wire_library", - ]: - targets.update(get_targets_for_type(bfa)) - return list(sorted(targets)) + skipped_targets_opt = f"--filter-type=-{','.join(self.skipped_target_types)}" + + pants_run = self.run_pants([ + 'filter', + skipped_targets_opt, + 'testprojects::', + 'examples::', + *exclude_opts + ]) + self.assert_success(pants_run) + return list(sorted(pants_run.stdout_data.split())) def targets_for_shard(self, shard): if shard < 0 or shard >= self._SHARDS: @@ -180,7 +167,9 @@ def targets_for_shard(self, shard): def run_shard(self, shard): targets = self.targets_for_shard(shard) - pants_run = self.run_pants(['test', *targets, '--jvm-platform-default-platform=java7']) + pants_run = self.run_pants([ + 'compile', 'lint', 'test', *targets, '--jvm-platform-default-platform=java7' + ]) self.assert_success(pants_run) def test_self(self): From 6bf228e65d4b0e0e7327b5070ae5cd1229a9b814 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Sat, 5 Oct 2019 08:54:46 -0700 Subject: [PATCH 5/7] fix lint --- .../python/pants_test/projects/test_testprojects_integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/python/pants_test/projects/test_testprojects_integration.py b/tests/python/pants_test/projects/test_testprojects_integration.py index 07587ad25a3..9b926deb39e 100644 --- a/tests/python/pants_test/projects/test_testprojects_integration.py +++ b/tests/python/pants_test/projects/test_testprojects_integration.py @@ -2,7 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import math -from typing import List, Set +from typing import List from pants.util.memo import memoized_property from pants_test.pants_run_integration_test import PantsRunIntegrationTest From abdb2141aabca1a1c0015553946e50a26b491c18 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Sat, 5 Oct 2019 10:02:57 -0700 Subject: [PATCH 6/7] No longer skip types that were causing issues: 1) managed_jar_libraries is not recognized 2) target means that _everything_ will be skipped --- .../pants_test/projects/test_testprojects_integration.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/python/pants_test/projects/test_testprojects_integration.py b/tests/python/pants_test/projects/test_testprojects_integration.py index 9b926deb39e..4435c9777b9 100644 --- a/tests/python/pants_test/projects/test_testprojects_integration.py +++ b/tests/python/pants_test/projects/test_testprojects_integration.py @@ -129,14 +129,12 @@ def skipped_target_types(self) -> List[str]: """We don't want to run over every single target, e.g. files() are only used for us to depend on in ITs and it doesn't make sense to run `./pants test` against them.""" return [ - "target", # resources / loose files "files", "resources", "page", # 3rd-party dependencies "jar_library", - "managed_jar_libraries", "python_requirement_library", ] @@ -154,6 +152,7 @@ def targets(self) -> List[str]: 'examples::', *exclude_opts ]) + import pdb; pdb.set_trace() self.assert_success(pants_run) return list(sorted(pants_run.stdout_data.split())) From 39e11fadbb22c0c3faca6c0261c60e176239d324 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Sat, 5 Oct 2019 11:09:43 -0700 Subject: [PATCH 7/7] Remove pdb. Oops! --- .../python/pants_test/projects/test_testprojects_integration.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/python/pants_test/projects/test_testprojects_integration.py b/tests/python/pants_test/projects/test_testprojects_integration.py index 4435c9777b9..a2d4495e08f 100644 --- a/tests/python/pants_test/projects/test_testprojects_integration.py +++ b/tests/python/pants_test/projects/test_testprojects_integration.py @@ -152,7 +152,6 @@ def targets(self) -> List[str]: 'examples::', *exclude_opts ]) - import pdb; pdb.set_trace() self.assert_success(pants_run) return list(sorted(pants_run.stdout_data.split()))