Merge pull request #3 from mndeveci/mingkun_build_strategy

Mingkun build strategy

samcli/lib/build/app_builder.py

@@ -16,7 +16,7 @@
 import samcli.lib.utils.osutils as osutils
 from samcli.lib.utils.colors import Colored
 from samcli.lib.providers.sam_base_provider import SamBaseProvider
-from samcli.lib.build.build_graph import LayerBuildDefinition, BuildGraph, FunctionBuildDefinition
+from samcli.lib.build.build_graph import FunctionBuildDefinition, LayerBuildDefinition, BuildGraph
 from samcli.lib.build.build_strategy import DefaultBuildStrategy, CachedBuildStrategy
 from samcli.local.docker.lambda_build_container import LambdaBuildContainer
 from .workflow_config import get_workflow_config, get_layer_subfolder, supports_build_in_container
@@ -124,17 +124,15 @@ def build(self):
             Returns the path to where each resource was built as a map of resource's LogicalId to the path string
         """
         build_graph = self._get_build_graph()
-        build_strategy = DefaultBuildStrategy(
-            build_graph,
-            self._build_dir,
-            self._resources_to_build,
-            self._is_building_specific_resource,
-            self._build_function,
-            self._build_layer,
-        )
+        build_strategy = DefaultBuildStrategy(build_graph, self._build_dir, self._build_function, self._build_layer)
 
         if self._cached:
-            build_strategy = CachedBuildStrategy(build_graph, build_strategy, self._base_dir, self._build_dir, self._cache_dir)
+            build_strategy = CachedBuildStrategy(build_graph,
+                                                 build_strategy,
+                                                 self._base_dir,
+                                                 self._build_dir,
+                                                 self._cache_dir,
+                                                 self._is_building_specific_resource)
 
         return build_strategy.build()
 

samcli/lib/build/build_strategy.py

@@ -38,12 +38,6 @@ def build(self):
 
         return result
 
-    def is_building_specific_resource(self):
-        """
-        Returns True if build is called for specific resource (Function or Layer)
-        """
-        return False
-
     def _build_functions(self, build_graph):
         """
         Iterates through build graph and runs each unique build and copies outcome to the corresponding function folder
@@ -84,42 +78,46 @@ class DefaultBuildStrategy(BuildStrategy):
     Default build strategy, loops over given build graph for each function and layer, and builds each of them one by one
     """
 
-    def __init__(self, build_graph, build_dir, resources_to_build, is_building_specific_resource, build_function,
-                 build_layer):
+    def __init__(self, build_graph, build_dir, build_function, build_layer):
         super().__init__(build_graph)
         self._build_dir = build_dir
-        self._resources_to_build = resources_to_build
-        self._is_building_specific_resource = is_building_specific_resource
         self._build_function = build_function
         self._build_layer = build_layer
 
-    def is_building_specific_resource(self):
-        return self._is_building_specific_resource
-
     def build_single_function_definition(self, build_definition):
         """
         Build the unique definition and then copy the artifact to the corresponding function folder
         """
-        function_results = {}
+        function_build_results = {}
         LOG.info("Building codeuri: %s runtime: %s metadata: %s functions: %s",
-                 build_definition.codeuri, build_definition.runtime, build_definition.metadata,
+                 build_definition.codeuri,
+                 build_definition.runtime,
+                 build_definition.metadata,
                  [function.name for function in build_definition.functions])
-        with osutils.mkdir_temp() as temporary_build_dir:
-            LOG.debug("Building to following folder %s", temporary_build_dir)
-            self._build_function(build_definition.get_function_name(),
-                                 build_definition.codeuri,
-                                 build_definition.runtime,
-                                 build_definition.get_handler_name(),
-                                 temporary_build_dir,
-                                 build_definition.metadata)
 
-            for function in build_definition.functions:
+        # build into one of the functions from this build definition
+        single_function_name = build_definition.get_function_name()
+        single_build_dir = str(pathlib.Path(self._build_dir, single_function_name))
+
+        LOG.debug("Building to following folder %s", single_build_dir)
+        self._build_function(build_definition.get_function_name(),
+                             build_definition.codeuri,
+                             build_definition.runtime,
+                             build_definition.get_handler_name(),
+                             single_build_dir,
+                             build_definition.metadata)
+        function_build_results[single_function_name] = single_build_dir
+
+        # copy results to other functions
+        for function in build_definition.functions:
+            if function.name is not single_function_name:
                 # artifacts directory will be created by the builder
                 artifacts_dir = str(pathlib.Path(self._build_dir, function.name))
-                LOG.debug("Copying artifacts from %s to %s", temporary_build_dir, artifacts_dir)
-                osutils.copytree(temporary_build_dir, artifacts_dir)
-                function_results[function.name] = artifacts_dir
-        return function_results
+                LOG.debug("Copying artifacts from %s to %s", single_build_dir, artifacts_dir)
+                osutils.copytree(single_build_dir, artifacts_dir)
+                function_build_results[function.name] = artifacts_dir
+
+        return function_build_results
 
     def build_single_layer_definition(self, layer_definition):
         """
@@ -145,12 +143,14 @@ class CachedBuildStrategy(BuildStrategy):
     For actual building, it uses delegate implementation
     """
 
-    def __init__(self, build_graph, delegate_build_strategy, base_dir, build_dir, cache_dir):
+    def __init__(self, build_graph, delegate_build_strategy, base_dir, build_dir, cache_dir,
+                 is_building_specific_resource):
         super().__init__(build_graph)
         self._delegate_build_strategy = delegate_build_strategy
         self._base_dir = base_dir
         self._build_dir = build_dir
         self._cache_dir = cache_dir
+        self._is_building_specific_resource = is_building_specific_resource
 
     def __exit__(self, exc_type, exc_val, exc_tb):
         self._clean_redundant_cached()
@@ -232,8 +232,7 @@ def _clean_redundant_cached(self):
         """
         clean the redundant cached folder
         """
-        self._build_graph.clean_redundant_definitions_and_update(
-            not self._delegate_build_strategy.is_building_specific_resource())
+        self._build_graph.clean_redundant_definitions_and_update(not self._is_building_specific_resource)
         uuids = {bd.uuid for bd in self._build_graph.get_function_build_definitions()}
         uuids.update({ld.uuid for ld in self._build_graph.get_layer_build_definitions()})
         for cache_dir in pathlib.Path(self._cache_dir).iterdir():

tests/unit/lib/build_module/test_app_builder.py

@@ -137,6 +137,38 @@ def test_should_run_build_for_only_unique_builds(self, persist_mock, read_mock):
             any_order=True,
         )
 
+    @patch("samcli.lib.build.app_builder.DefaultBuildStrategy")
+    def test_default_run_should_pick_default_strategy(self, mock_default_build_strategy_class):
+        mock_default_build_strategy = Mock()
+        mock_default_build_strategy_class.return_value = mock_default_build_strategy
+
+        build_graph_mock = Mock()
+        get_build_graph_mock = Mock(return_value=build_graph_mock)
+
+        builder = ApplicationBuilder(Mock(), "builddir", "basedir", "cachedir")
+        builder._get_build_graph = get_build_graph_mock
+
+        result = builder.build()
+
+        mock_default_build_strategy.build.assert_called_once()
+        self.assertEqual(result, mock_default_build_strategy.build())
+
+    @patch("samcli.lib.build.app_builder.CachedBuildStrategy")
+    def test_cached_run_should_pick_cached_strategy(self, mock_cached_build_strategy_class):
+        mock_cached_build_strategy = Mock()
+        mock_cached_build_strategy_class.return_value = mock_cached_build_strategy
+
+        build_graph_mock = Mock()
+        get_build_graph_mock = Mock(return_value=build_graph_mock)
+
+        builder = ApplicationBuilder(Mock(), "builddir", "basedir", "cachedir", cached=True)
+        builder._get_build_graph = get_build_graph_mock
+
+        result = builder.build()
+
+        mock_cached_build_strategy.build.assert_called_once()
+        self.assertEqual(result, mock_cached_build_strategy.build())
+
 
 class TestApplicationBuilderForLayerBuild(TestCase):
     def setUp(self):

tests/unit/lib/build_module/test_build_graph.py

@@ -101,8 +101,8 @@ def test_should_instantiate_first_time(self):
             build_graph2 = BuildGraph(str(build_dir.resolve()))
 
             self.assertEqual(
-                build_graph1.get_function_build_definitions(), build_graph2.get_function_build_definitions()
-            )
+                build_graph1.get_function_build_definitions(),
+                build_graph2.get_function_build_definitions())
 
     def test_should_instantiate_first_time_and_update(self):
         with osutils.mkdir_temp() as temp_base_dir:
@@ -123,11 +123,11 @@ def test_should_instantiate_first_time_and_update(self):
             # read previously persisted graph and compare
             build_graph2 = BuildGraph(str(build_dir))
             self.assertEqual(
-                len(build_graph1.get_function_build_definitions()), len(build_graph2.get_function_build_definitions())
-            )
+                len(build_graph1.get_function_build_definitions()),
+                len(build_graph2.get_function_build_definitions()))
             self.assertEqual(
                 list(build_graph1.get_function_build_definitions())[0],
-                list(build_graph2.get_function_build_definitions())[0],
+                list(build_graph2.get_function_build_definitions())[0]
             )
 
     def test_should_read_existing_build_graph(self):
@@ -169,9 +169,7 @@ def test_functions_should_be_added_existing_build_graph(self):
                 self.assertTrue(build_definition.functions[0], function1)
                 self.assertEqual(build_definition.uuid, TestBuildGraph.UUID)
 
-            build_definition2 = FunctionBuildDefinition(
-                "another_runtime", "another_codeuri", None, "another_source_md5"
-            )
+            build_definition2 = FunctionBuildDefinition("another_runtime", "another_codeuri", None, "another_source_md5")
             function2 = generate_function(name="another_function")
             build_graph.put_function_build_definition(build_definition2, function2)
             self.assertTrue(len(build_graph.get_function_build_definitions()), 2)

tests/unit/lib/build_module/test_build_strategy.py

@@ -1,18 +1,166 @@
 from unittest import TestCase
-from unittest.mock import Mock, patch
-from pathlib import Path
+from unittest.mock import Mock, patch, MagicMock, call, ANY
 
-from samcli.lib.build.build_graph import BuildGraph
-from samcli.lib.build.build_strategy import CachedBuildStrategy, DefaultBuildStrategy
+from samcli.commands.build.exceptions import MissingBuildMethodException
+from samcli.lib.build.build_graph import BuildGraph, FunctionBuildDefinition, LayerBuildDefinition
+from samcli.lib.build.build_strategy import BuildStrategy, DefaultBuildStrategy, CachedBuildStrategy
 from samcli.lib.utils import osutils
+from pathlib import Path
+
+
+@patch("samcli.lib.build.build_graph.BuildGraph._write")
+@patch("samcli.lib.build.build_graph.BuildGraph._read")
+class BuildStrategyBaseTest(TestCase):
+
+    def setUp(self):
+        # create a build graph with 2 function definitions and 2 layer definitions
+        self.build_graph = BuildGraph("build_dir")
+
+        self.function1_1 = Mock()
+        self.function1_2 = Mock()
+        self.function2 = Mock()
+
+        self.function_build_definition1 = FunctionBuildDefinition("runtime", "codeuri", {})
+        self.function_build_definition1.functions = [self.function1_1, self.function1_2]
+        self.function_build_definition2 = FunctionBuildDefinition("runtime2", "codeuri", {})
+        self.function_build_definition1.functions = [self.function2]
+        self.build_graph.put_function_build_definition(self.function_build_definition1, Mock())
+        self.build_graph.put_function_build_definition(self.function_build_definition2, Mock())
+
+        self.layer_build_definition1 = LayerBuildDefinition("layer1", "codeuri", "build_method", [])
+        self.layer_build_definition2 = LayerBuildDefinition("layer2", "codeuri", "build_method", [])
+        self.build_graph.put_layer_build_definition(self.layer_build_definition1, Mock())
+        self.build_graph.put_layer_build_definition(self.layer_build_definition2, Mock())
+
+
+class BuildStrategyTest(BuildStrategyBaseTest):
+
+    def test_build_functions_layers(self):
+        build_strategy = BuildStrategy(self.build_graph)
+
+        self.assertEqual(build_strategy.build(), {})
+
+    def test_build_functions_layers_mock(self):
+        mock_build_strategy = BuildStrategy(self.build_graph)
+        given_build_functions_result = {"function1": "build_dir_1"}
+        given_build_layers_result = {"layer1": "layer_dir_1"}
+
+        mock_build_strategy._build_functions = Mock(return_value=given_build_functions_result)
+        mock_build_strategy._build_layers = Mock(return_value=given_build_layers_result)
+        build_result = mock_build_strategy.build()
+
+        expected_result = {}
+        expected_result.update(given_build_functions_result)
+        expected_result.update(given_build_layers_result)
+        self.assertEqual(build_result, expected_result)
+
+        mock_build_strategy._build_functions.assert_called_once_with(self.build_graph)
+        mock_build_strategy._build_layers.assert_called_once_with(self.build_graph)
+
+    def test_build_single_function_layer(self):
+        mock_build_strategy = BuildStrategy(self.build_graph)
+        given_build_functions_result = [{"function1": "build_dir_1"}, {"function2": "build_dir_2"}]
+        given_build_layers_result = [{"layer1": "layer_dir_1"}, {"layer2": "layer_dir_2"}]
+
+        mock_build_strategy.build_single_function_definition = Mock(side_effect=given_build_functions_result)
+        mock_build_strategy.build_single_layer_definition = Mock(side_effect=given_build_layers_result)
+        build_result = mock_build_strategy.build()
+
+        expected_result = {}
+        for function_result in given_build_functions_result:
+            expected_result.update(function_result)
+        for layer_result in given_build_layers_result:
+            expected_result.update(layer_result)
+
+        self.assertEqual(build_result, expected_result)
+
+        # assert individual functions builds have been called
+        mock_build_strategy.build_single_function_definition.assert_has_calls([
+            call(self.function_build_definition1),
+            call(self.function_build_definition2),
+        ])
+
+        # assert individual layer builds have been called
+        mock_build_strategy.build_single_layer_definition.assert_has_calls([
+            call(self.layer_build_definition1),
+            call(self.layer_build_definition2),
+        ])
+
+
+@patch("samcli.lib.build.build_strategy.pathlib.Path")
+@patch("samcli.lib.build.build_strategy.osutils.copytree")
+class DefaultBuildStrategyTest(BuildStrategyBaseTest):
+    def test_layer_build_should_fail_when_no_build_method_is_provided(self, mock_copy_tree, mock_path):
+        given_layer = Mock()
+        given_layer.build_method = None
+        layer_build_definition = LayerBuildDefinition("layer1", "codeuri", "build_method", [])
+        layer_build_definition.layer = given_layer
+
+        build_graph = Mock(spec=BuildGraph)
+        build_graph.get_layer_build_definitions.return_value = [layer_build_definition]
+        build_graph.get_function_build_definitions.return_value = []
+        default_build_strategy = DefaultBuildStrategy(build_graph, "build_dir", Mock(), Mock())
+
+        self.assertRaises(MissingBuildMethodException, default_build_strategy.build)
+
+    def test_build_layers_and_functions(self, mock_copy_tree, mock_path):
+        given_build_function = Mock()
+        given_build_layer = Mock()
+        given_build_dir = "build_dir"
+        default_build_strategy = DefaultBuildStrategy(self.build_graph,
+                                                      given_build_dir,
+                                                      given_build_function,
+                                                      given_build_layer)
+
+        default_build_strategy.build()
 
+        # assert that build function has been called
+        given_build_function.assert_has_calls([
+            call(
+                self.function_build_definition1.get_function_name(),
+                self.function_build_definition1.codeuri,
+                self.function_build_definition1.runtime,
+                self.function_build_definition1.get_handler_name(),
+                ANY,
+                self.function_build_definition1.metadata
+            ),
+            call(
+                self.function_build_definition2.get_function_name(),
+                self.function_build_definition2.codeuri,
+                self.function_build_definition2.runtime,
+                self.function_build_definition2.get_handler_name(),
+                ANY,
+                self.function_build_definition2.metadata
+            ),
+        ])
 
-class BuildStrategyTest(TestCase):
-    pass
+        # assert that layer build function has been called
+        given_build_layer.assert_has_calls([
+            call(
+                self.layer_build_definition1.layer.name,
+                self.layer_build_definition1.layer.codeuri,
+                self.layer_build_definition1.layer.build_method,
+                self.layer_build_definition1.layer.compatible_runtimes,
+            ),
+            call(
+                self.layer_build_definition2.layer.name,
+                self.layer_build_definition2.layer.codeuri,
+                self.layer_build_definition2.layer.build_method,
+                self.layer_build_definition2.layer.compatible_runtimes,
+            ),
+        ])
 
+        # assert that mock path has been called
+        mock_path.assert_has_calls([
+            call(given_build_dir, self.function_build_definition1.get_function_name()),
+            call(given_build_dir, self.function_build_definition2.get_function_name()),
+        ], any_order=True)
 
-class DefaultBuildStrategyTest(TestCase):
-    pass
+        # assert that function1_2 artifacts have been copied from already built function1_1
+        mock_copy_tree.assert_called_with(
+            str(mock_path(given_build_dir, self.function_build_definition1.get_function_name())),
+            str(mock_path(given_build_dir, self.function1_2.name))
+        )
 
 
 class CachedBuildStrategyTest(TestCase):