Skip to content

Commit

Permalink
Add basic native task unit tests.
Browse files Browse the repository at this point in the history
Running a `./pants lint` across the whole repo revealed an issue
accessing a non-existant `self.linker.platform` attribute in
`LinkSharedLibraries`. Add basic unit tests to exercise task execution
with a full cache miss and then a full hit.
  • Loading branch information
jsirois committed Jul 26, 2018
1 parent 5f9adf2 commit 5258fc9
Show file tree
Hide file tree
Showing 10 changed files with 213 additions and 24 deletions.
21 changes: 12 additions & 9 deletions src/python/pants/backend/native/tasks/link_shared_libraries.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def product_types(cls):

@classmethod
def prepare(cls, options, round_manager):
super(LinkSharedLibraries, cls).prepare(options, round_manager)
round_manager.require(NativeTargetDependencies)
round_manager.require(ObjectFiles)
round_manager.require(NativeExternalLibraryFetch.NativeExternalLibraryFiles)
Expand Down Expand Up @@ -70,6 +71,11 @@ def _cpp_toolchain(self):
def linker(self):
return self._cpp_toolchain.cpp_linker

@memoized_property
def platform(self):
# FIXME: convert this to a v2 engine dependency injection.
return Platform.create()

def _retrieve_single_product_at_target_base(self, product_mapping, target):
self.context.log.debug("product_mapping: {}".format(product_mapping))
self.context.log.debug("target: {}".format(target))
Expand All @@ -87,14 +93,11 @@ def execute(self):

all_shared_libs_by_name = {}

# FIXME: convert this to a v2 engine dependency injection.
platform = Platform.create()

with self.invalidated(targets_providing_artifacts,
invalidate_dependents=True) as invalidation_check:
for vt in invalidation_check.all_vts:
if vt.valid:
shared_library = self._retrieve_shared_lib_from_cache(vt, platform)
shared_library = self._retrieve_shared_lib_from_cache(vt)
else:
# FIXME: We need to partition links based on proper dependency edges and not
# perform a link to every native_external_library for all targets in the closure.
Expand All @@ -117,10 +120,10 @@ def execute(self):

shared_libs_product.add(vt.target, vt.target.target_base).append(shared_library)

def _retrieve_shared_lib_from_cache(self, vt, platform):
def _retrieve_shared_lib_from_cache(self, vt):
native_artifact = vt.target.ctypes_native_library
path_to_cached_lib = os.path.join(
vt.results_dir, native_artifact.as_shared_lib(platform))
vt.results_dir, native_artifact.as_shared_lib(self.platform))
if not os.path.isfile(path_to_cached_lib):
raise self.LinkSharedLibrariesError("The shared library at {} does not exist!"
.format(path_to_cached_lib))
Expand Down Expand Up @@ -164,14 +167,14 @@ def _execute_link_request(self, link_request):
raise self.LinkSharedLibrariesError("No object files were provided in request {}!"
.format(link_request))

platform = Platform.create()
linker = link_request.linker
native_artifact = link_request.native_artifact
output_dir = link_request.output_dir
resulting_shared_lib_path = os.path.join(output_dir, native_artifact.as_shared_lib(platform))
resulting_shared_lib_path = os.path.join(output_dir,
native_artifact.as_shared_lib(self.platform))
# We are executing in the results_dir, so get absolute paths for everything.
cmd = ([linker.exe_filename] +
platform.resolve_platform_specific(self._SHARED_CMDLINE_ARGS) +
self.platform.resolve_platform_specific(self._SHARED_CMDLINE_ARGS) +
linker.extra_args +
link_request.external_libs_info.get_third_party_lib_args() +
['-o', os.path.abspath(resulting_shared_lib_path)] +
Expand Down
10 changes: 8 additions & 2 deletions src/python/pants/backend/native/tasks/native_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from pants.backend.native.targets.native_library import NativeLibrary
from pants.backend.native.tasks.native_external_library_fetch import NativeExternalLibraryFetch
from pants.backend.native.tasks.native_task import NativeTask
from pants.base.build_environment import get_buildroot
from pants.base.exceptions import TaskError
from pants.base.workunit import WorkUnit, WorkUnitLabel
from pants.build_graph.dependency_context import DependencyContext
Expand Down Expand Up @@ -60,6 +61,11 @@ class NativeCompile(NativeTask, AbstractClass):
def product_types(cls):
return [ObjectFiles, NativeTargetDependencies]

@classmethod
def prepare(cls, options, round_manager):
super(NativeCompile, cls).prepare(options, round_manager)
round_manager.require(NativeExternalLibraryFetch.NativeExternalLibraryFiles)

@property
def cache_target_dirs(self):
return True
Expand Down Expand Up @@ -138,7 +144,7 @@ def execute(self):
# This may be calculated many times for a target, so we memoize it.
@memoized_method
def _include_dirs_for_target(self, target):
return target.sources_relative_to_target_base().rel_root
return os.path.join(get_buildroot(), target.target_base)

class NativeSourcesByType(datatype(['rel_root', 'headers', 'sources'])): pass

Expand Down Expand Up @@ -172,7 +178,7 @@ def get_sources_headers_for_target(self, target):
"Conflicting filenames:\n{}"
.format(target.address.spec, target.alias(), '\n'.join(duplicate_filename_err_msgs)))

return [os.path.join(rel_root, src) for src in target_relative_sources]
return [os.path.join(get_buildroot(), rel_root, src) for src in target_relative_sources]

# FIXME(#5951): expand `Executable` to cover argv generation (where an `Executable` is subclassed
# to modify or extend the argument list, as declaratively as possible) to remove
Expand Down
4 changes: 1 addition & 3 deletions src/python/pants/init/engine_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ def setup_legacy_graph(native, bootstrap_options, build_configuration):
build_configuration,
native=native,
glob_match_error_behavior=bootstrap_options.glob_expansion_failure,
rules=build_configuration.rules(),
build_ignore_patterns=bootstrap_options.build_ignore,
exclude_target_regexps=bootstrap_options.exclude_target_regexp,
subproject_roots=bootstrap_options.subproject_roots,
Expand All @@ -271,7 +270,6 @@ def setup_legacy_graph_extended(
build_root=None,
native=None,
glob_match_error_behavior=None,
rules=None,
build_ignore_patterns=None,
exclude_target_regexps=None,
subproject_roots=None,
Expand Down Expand Up @@ -308,7 +306,7 @@ def setup_legacy_graph_extended(
build_root = build_root or get_buildroot()
build_configuration = build_configuration or BuildConfigInitializer.get(OptionsBootstrapper())
build_file_aliases = build_configuration.registered_aliases()
rules = rules or build_configuration.rules() or []
rules = build_configuration.rules()
console = Console()

symbol_table = LegacySymbolTable(build_file_aliases)
Expand Down
18 changes: 18 additions & 0 deletions tests/python/pants_test/backend/native/tasks/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
python_tests(
dependencies=[
':native_task_test_base',
'src/python/pants/backend/native/targets',
'src/python/pants/backend/native/tasks',
],
tags={'platform_specific_behavior'},
)

python_library(
name='native_task_test_base',
sources=['native_task_test_base.py'],
dependencies=[
'src/python/pants/backend/native',
'src/python/pants/backend/native/targets',
'tests/python/pants_test:task_test_base',
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

import os
from textwrap import dedent

from pants.backend.native import register
from pants.backend.native.targets.native_library import CppLibrary
from pants.backend.native.tasks.native_external_library_fetch import NativeExternalLibraryFetch
from pants_test.task_test_base import TaskTestBase


class NativeTaskTestBase(TaskTestBase):
@classmethod
def rules(cls):
return super(NativeTaskTestBase, cls).rules() + register.rules()


class NativeCompileTestMixin(object):
def create_simple_cpp_library(self, **kwargs):
self.create_file('src/cpp/test/test.hpp', contents=dedent("""
#ifndef __TEST_HPP__
#define __TEST_HPP__
int test(int);
extern "C" int test_exported(int);
#endif
"""))
self.create_file('src/cpp/test/test.cpp', contents=dedent("""
#include "test.hpp"
int test(int x) {
return x / 137;
}
extern "C" int test_exported(int x) {
return test(x * 42);
}
"""))
return self.make_target(spec='src/cpp/test',
target_type=CppLibrary,
sources=['test.hpp', 'test.cpp'],
**kwargs)

def prepare_context_for_compile(self, target_roots, for_task_types=None, **kwargs):
native_elf_fetch_task_type = self.synthesize_task_subtype(NativeExternalLibraryFetch,
'native_elf_fetch_scope')

for_task_types = list(for_task_types or ()) + [native_elf_fetch_task_type]
context = self.context(target_roots=target_roots, for_task_types=for_task_types, **kwargs)

native_elf_fetch = native_elf_fetch_task_type(context,
os.path.join(self.pants_workdir,
'native_elf_fetch'))
native_elf_fetch.execute()
return context
47 changes: 47 additions & 0 deletions tests/python/pants_test/backend/native/tasks/test_c_compile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

from textwrap import dedent

from pants.backend.native.targets.native_library import CLibrary
from pants.backend.native.tasks.c_compile import CCompile
from pants_test.backend.native.tasks.native_task_test_base import (NativeCompileTestMixin,
NativeTaskTestBase)


class CCompileTest(NativeTaskTestBase, NativeCompileTestMixin):
@classmethod
def task_type(cls):
return CCompile

def create_simple_c_library(self, **kwargs):
self.create_file('src/c/test/test.h', contents=dedent("""
#ifndef __TEST_H__
#define __TEST_H__
int test(int);
#endif
"""))
self.create_file('src/c/test/test.c', contents=dedent("""
#include "test.h"
int test(int x) {
return x / 137;
}
"""))
return self.make_target(spec='src/c/test',
target_type=CLibrary,
sources=['test.h', 'test.c'],
**kwargs)

def test_caching(self):
c = self.create_simple_c_library()
context = self.prepare_context_for_compile(target_roots=[c])
c_compile = self.create_task(context)

c_compile.execute()
c_compile.execute()
23 changes: 23 additions & 0 deletions tests/python/pants_test/backend/native/tasks/test_cpp_compile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

from pants.backend.native.tasks.cpp_compile import CppCompile
from pants_test.backend.native.tasks.native_task_test_base import (NativeCompileTestMixin,
NativeTaskTestBase)


class CppCompileTest(NativeTaskTestBase, NativeCompileTestMixin):
@classmethod
def task_type(cls):
return CppCompile

def test_caching(self):
cpp = self.create_simple_cpp_library()
context = self.prepare_context_for_compile(target_roots=[cpp])
cpp_compile = self.create_task(context)

cpp_compile.execute()
cpp_compile.execute()
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

import os

from pants.backend.native.targets.native_artifact import NativeArtifact
from pants.backend.native.tasks.cpp_compile import CppCompile
from pants.backend.native.tasks.link_shared_libraries import LinkSharedLibraries
from pants_test.backend.native.tasks.native_task_test_base import (NativeCompileTestMixin,
NativeTaskTestBase)


class LinkSharedLibrariesTest(NativeTaskTestBase, NativeCompileTestMixin):
@classmethod
def task_type(cls):
return LinkSharedLibraries

def test_caching(self):
cpp = self. create_simple_cpp_library(ctypes_native_library=NativeArtifact(lib_name='test'),)

cpp_compile_task_type = self.synthesize_task_subtype(CppCompile, 'cpp_compile_scope')
context = self.prepare_context_for_compile(target_roots=[cpp],
for_task_types=[cpp_compile_task_type])

cpp_compile = cpp_compile_task_type(context, os.path.join(self.pants_workdir, 'cpp_compile'))
cpp_compile.execute()

link_shared_libraries = self.create_task(context)

link_shared_libraries.execute()
link_shared_libraries.execute()
6 changes: 3 additions & 3 deletions tests/python/pants_test/engine/test_isolated_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,12 @@ def test_create_from_snapshot_with_env(self):
class IsolatedProcessTest(TestBase, unittest.TestCase):

@classmethod
def extra_rules(cls):
return create_cat_stdout_rules() + create_javac_compile_rules() + [
def rules(cls):
return super(IsolatedProcessTest, cls).rules() + [
RootRule(JavacVersionExecutionRequest),
process_request_from_javac_version,
get_javac_version_output,
]
] + create_cat_stdout_rules() + create_javac_compile_rules()

def test_integration_concat_with_snapshots_stdout(self):

Expand Down
13 changes: 6 additions & 7 deletions tests/python/pants_test/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,16 @@ def alias_groups(cls):
"""
return BuildFileAliases(targets={'target': Target})

@classmethod
def rules(cls):
# Required for sources_for:
return [RootRule(SourcesField)]

@classmethod
def build_config(cls):
build_config = BuildConfiguration()
build_config.register_aliases(cls.alias_groups())
build_config.register_rules(cls.rules())
return build_config

def setUp(self):
Expand Down Expand Up @@ -355,11 +361,6 @@ def _build_root(cls):
def _pants_workdir(cls):
return os.path.join(cls._build_root(), '.pants.d')

@classmethod
def extra_rules(cls):
"""Override this to register extra rules in this class's scheduler."""
return []

@classmethod
def _init_engine(cls):
if cls._scheduler is not None:
Expand All @@ -374,8 +375,6 @@ def _init_engine(cls):
native=init_native(),
build_configuration=cls.build_config(),
build_ignore_patterns=None,
# Required for sources_for:
rules=cls.extra_rules() + [RootRule(SourcesField)],
).new_session()
cls._scheduler = graph_session.scheduler_session
cls._build_graph, cls._address_mapper = graph_session.create_build_graph(
Expand Down

0 comments on commit 5258fc9

Please sign in to comment.