Skip to content

Commit

Permalink
Exclude Antlr test from testprojects to avoid interpreter conflict (#…
Browse files Browse the repository at this point in the history
…6944)

### Problem
Antlr 3 only works with Python 2, so we have to constrain that part to only using Python 2, as done in #6924.

However, this results in one of the `testprojects` shards failing when using Python 3 due to mixing a Py2-only constraint with a Py3-only constraint.

```bash
$ export PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS='["CPython>=3.5,<4"]'
$ ./pants clean-all test testprojects/src/python/antlr:eval testprojects/src/python/antlr:eval-bin testprojects/src/python/coordinated_runs:creator testprojects/src/python/coordinated_runs:phaser testprojects/src/python/coordinated_runs:waiter testprojects/src/python/print_env:print_env

...

18:47:02 00:04   [pyprep]
18:47:02 00:04     [interpreter]
                   Invalidated 4 targets.
FAILURE: Unable to detect a suitable interpreter for compatibilities: CPython>=2.7,<3 && CPython>=3.5,<4 (Conflicting targets: testprojects/src/python/antlr:eval, testprojects/src/python/coordinated_runs:creator)
```

We can get Antlr to evaluate correctly when there are no other targets, but `test_testprojects_integration.py` works by running multiple targets with one single command so this does not help.

```bash
$ export PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS='["CPython>=3.5,<4"]'
$ ./pants clean-all test testprojects/src/python/antlr:eval testprojects/src/python/antlr:eval-bin 

{interpreter will resolve correctly}
```

### Solution
Exclude the Antlr testproject from `projects/testprojects`. The code is already covered by the tests in `backend/codegen/antlr/test_antlr_py_gen_integration.py`.

This is a much simpler solution than trying to come up with logic in `test_testprojects_integration.py` to handle interpreter conflicts like this, e.g. putting Python 2 only targets in one dedicated shard.

### Also fixes duplicate .so binary names
By removing the antlr test, the shards were redistributed in `test_testprojects_integration.py`. This led to `python_distribution/ctypes` and `python_distribution/ctypes_with_extra_compiler_flags` running at the same time, which resulted in the error `FAILURE: The name 'asdf-cpp' was used for two shared libraries`.

We fix this by making the `asdf-cpp` name unique for all of the 3 uses.
  • Loading branch information
Eric-Arellano committed Dec 17, 2018
1 parent 9b8528b commit dbc7ef9
Show file tree
Hide file tree
Showing 11 changed files with 19 additions and 13 deletions.
4 changes: 2 additions & 2 deletions testprojects/src/python/python_distribution/ctypes/BUILD
Expand Up @@ -4,13 +4,13 @@
ctypes_compatible_c_library(
name='c_library',
sources=['some_math.h', 'some_math.c', 'src-subdir/add_three.h', 'src-subdir/add_three.c'],
ctypes_native_library=native_artifact(lib_name='asdf-c'),
ctypes_native_library=native_artifact(lib_name='asdf-c_ctypes'),
)

ctypes_compatible_cpp_library(
name='cpp_library',
sources=['some_more_math.hpp', 'some_more_math.cpp'],
ctypes_native_library=native_artifact(lib_name='asdf-cpp'),
ctypes_native_library=native_artifact(lib_name='asdf-cpp_ctypes'),
)

python_dist(
Expand Down
Expand Up @@ -16,8 +16,8 @@ def get_generated_shared_lib(lib_name):
return os.path.normpath(rel_path)


asdf_c_lib_path = get_generated_shared_lib('asdf-c')
asdf_cpp_lib_path = get_generated_shared_lib('asdf-cpp')
asdf_c_lib_path = get_generated_shared_lib('asdf-c_ctypes')
asdf_cpp_lib_path = get_generated_shared_lib('asdf-cpp_ctypes')

asdf_c_lib = ctypes.CDLL(asdf_c_lib_path)
asdf_cpp_lib = ctypes.CDLL(asdf_cpp_lib_path)
Expand Down
Expand Up @@ -12,5 +12,5 @@
version='0.0.1',
packages=find_packages(),
# Declare two files at the top-level directory (denoted by '').
data_files=[('', ['libasdf-c.so', 'libasdf-cpp.so'])],
data_files=[('', ['libasdf-c_ctypes.so', 'libasdf-cpp_ctypes.so'])],
)
Expand Up @@ -4,7 +4,7 @@
ctypes_compatible_cpp_library(
name='cpp_library',
sources=['some_more_math.hpp', 'some_more_math.cpp'],
ctypes_native_library=native_artifact(lib_name='asdf-cpp'),
ctypes_native_library=native_artifact(lib_name='asdf-cpp_ctypes-with-extra-compiler-flags'),
compiler_option_sets={'asdf'},
)

Expand Down
Expand Up @@ -16,7 +16,7 @@ def get_generated_shared_lib(lib_name):
return os.path.normpath(rel_path)


asdf_cpp_lib_path = get_generated_shared_lib('asdf-cpp')
asdf_cpp_lib_path = get_generated_shared_lib('asdf-cpp_ctypes-with-extra-compiler-flags')
asdf_cpp_lib = ctypes.CDLL(asdf_cpp_lib_path)

def f(x):
Expand Down
Expand Up @@ -11,5 +11,5 @@
name='ctypes_test',
version='0.0.1',
packages=find_packages(),
data_files=[('', ['libasdf-cpp.so'])],
data_files=[('', ['libasdf-cpp_ctypes-with-extra-compiler-flags.so'])],
)
Expand Up @@ -6,7 +6,7 @@
ctypes_compatible_cpp_library(
name='cpp_library_with_third_party',
sources=['some_more_math.hpp', 'some_more_math_with_third_party.cpp'],
ctypes_native_library=native_artifact(lib_name='asdf-cpp-tp'),
ctypes_native_library=native_artifact(lib_name='asdf-cpp_ctypes-with-third-party'),
dependencies=[':rang', ':cereal'],
fatal_warnings=False
)
Expand Down
Expand Up @@ -16,7 +16,7 @@ def get_generated_shared_lib(lib_name):
return os.path.normpath(rel_path)


asdf_cpp_lib_path = get_generated_shared_lib('asdf-cpp-tp')
asdf_cpp_lib_path = get_generated_shared_lib('asdf-cpp_ctypes-with-third-party')
asdf_cpp_lib = ctypes.CDLL(asdf_cpp_lib_path)

def f(x):
Expand Down
Expand Up @@ -12,5 +12,5 @@
version='0.0.1',
packages=find_packages(),
# Declare one shared lib at the top-level directory (denoted by '').
data_files=[('', ['libasdf-cpp-tp.so'])],
data_files=[('', ['libasdf-cpp_ctypes-with-third-party.so'])],
)
Expand Up @@ -108,7 +108,7 @@ def _assert_ctypes_binary_creation(self, toolchain_variant):
wheel_files = ZipFile(wheel_dist_with_path).namelist()

dist_versioned_name = '{}-{}.data'.format(dist_name, dist_version)
for shared_lib_filename in ['libasdf-c.so', 'libasdf-cpp.so']:
for shared_lib_filename in ['libasdf-c_ctypes.so', 'libasdf-cpp_ctypes.so']:
full_path_in_wheel = os.path.join(dist_versioned_name, 'data', shared_lib_filename)
self.assertIn(full_path_in_wheel, wheel_files)

Expand Down
Expand Up @@ -80,6 +80,12 @@ def targets(self):
'examples/src/java/org/pantsbuild/example/plugin',
]

# 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'
]

# Targets for testing timeouts. These should only be run during specific integration tests,
# because they take a long time to run.
timeout_targets = [
Expand All @@ -104,7 +110,7 @@ def targets(self):
'testprojects/src/python/python_distribution/ctypes_with_extra_compiler_flags:bin',
]

targets_to_exclude = (known_failing_targets + negative_test_targets + need_java_8 +
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]

Expand Down

0 comments on commit dbc7ef9

Please sign in to comment.