From 5084944c79b8f73fa362700fd7f31c783b8cb4ee Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Tue, 17 Jan 2023 16:19:04 -0800 Subject: [PATCH 1/6] refactor: assign each workflow a default build directory --- aws_lambda_builders/workflow.py | 55 ++++++--- .../workflows/custom_make/workflow.py | 21 +--- .../workflows/dotnet_clipackage/workflow.py | 4 +- .../workflows/go_modules/workflow.py | 4 +- .../workflows/java_gradle/workflow.py | 4 +- .../workflows/java_maven/workflow.py | 4 +- .../workflows/nodejs_npm/workflow.py | 4 +- .../workflows/nodejs_npm_esbuild/workflow.py | 4 +- .../workflows/python_pip/workflow.py | 4 +- .../workflows/ruby_bundler/workflow.py | 4 +- .../workflows/hello_workflow/write_hello.py | 4 +- tests/unit/test_builder.py | 4 +- tests/unit/test_workflow.py | 107 +++++++++++++----- .../workflows/custom_make/test_workflow.py | 7 +- 14 files changed, 148 insertions(+), 82 deletions(-) diff --git a/aws_lambda_builders/workflow.py b/aws_lambda_builders/workflow.py index f0dfb9ebc..9a1447759 100644 --- a/aws_lambda_builders/workflow.py +++ b/aws_lambda_builders/workflow.py @@ -7,6 +7,7 @@ from collections import namedtuple from enum import Enum +from typing import Dict, Optional from aws_lambda_builders.binary_path import BinaryPath from aws_lambda_builders.path_resolver import PathResolver @@ -37,6 +38,10 @@ class BuildMode(object): DEBUG = "debug" RELEASE = "release" +class BuildDirectory(Enum): + SCRATCH = "scratch" + ARTIFACTS = "artifacts" + SOURCE = "source" class BuildInSourceSupport(Enum): """ @@ -140,13 +145,16 @@ def __new__(mcs, name, bases, class_dict): if not isinstance(cls.CAPABILITY, Capability): raise ValueError("Workflow '{}' must register valid capabilities".format(cls.NAME)) - # All workflows must define valid default and supported values for build in source - if ( - not isinstance(cls.BUILD_IN_SOURCE_SUPPORT, BuildInSourceSupport) - or cls.BUILD_IN_SOURCE_BY_DEFAULT not in cls.BUILD_IN_SOURCE_SUPPORT.value - ): + # All workflows define supported values for build in source + if not isinstance(cls.BUILD_IN_SOURCE_SUPPORT, BuildInSourceSupport): raise ValueError( - "Workflow '{}' must define valid default and supported values for build in source".format(cls.NAME) + "Workflow '{}' must define supported values for build in source".format(cls.NAME) + ) + + # All workflows define default build directory + if not isinstance(cls.DEFAULT_BUILD_DIR, BuildDirectory): + raise ValueError( + "Workflow '{}' must define default build directory".format(cls.NAME) ) LOG.debug("Registering workflow '%s' with capability '%s'", cls.NAME, cls.CAPABILITY) @@ -173,12 +181,12 @@ class BaseWorkflow(object, metaclass=_WorkflowMetaClass): # Optional list of manifests file/folder names supported by this workflow. SUPPORTED_MANIFESTS = [] - # Whether the workflow builds in source by default, each workflow should define this. - # (some workflows build in temporary or artifact directories by default) - BUILD_IN_SOURCE_BY_DEFAULT = None # Support for building in source, each workflow should define this. BUILD_IN_SOURCE_SUPPORT = None + # Default build directory, each workflow should define this. + DEFAULT_BUILD_DIR = None + def __init__( self, source_dir, @@ -260,22 +268,39 @@ def __init__( self.architecture = architecture self.is_building_layer = is_building_layer self.experimental_flags = experimental_flags if experimental_flags else [] + self.build_dir = self._select_build_dir(build_in_source) + + # Actions are registered by the subclasses as they seem fit + self.actions = [] + self._binaries = {} - self.build_in_source = build_in_source + + def _select_build_dir(self, build_in_source: Optional[bool]) -> str: + """ + Returns the build directory for the workflow. + """ + + should_build_in_source = build_in_source if build_in_source not in self.BUILD_IN_SOURCE_SUPPORT.value: + # assign default value + should_build_in_source = self.DEFAULT_BUILD_DIR == BuildDirectory.SOURCE + # only show warning if an unsupported value was explicitly passed in if build_in_source is not None: LOG.warning( 'Workflow %s does not support value "%s" for building in source. Using default value "%s".', self.NAME, build_in_source, - self.BUILD_IN_SOURCE_BY_DEFAULT, + should_build_in_source, ) - self.build_in_source = self.BUILD_IN_SOURCE_BY_DEFAULT - # Actions are registered by the subclasses as they seem fit - self.actions = [] - self._binaries = {} + build_directory_mapping = { + BuildDirectory.SCRATCH: self.scratch_dir, + BuildDirectory.ARTIFACTS: self.artifacts_dir, + BuildDirectory.SOURCE: self.source_dir, + } + + return self.source_dir if should_build_in_source else build_directory_mapping.get(self.DEFAULT_BUILD_DIR) def is_supported(self): """ diff --git a/aws_lambda_builders/workflows/custom_make/workflow.py b/aws_lambda_builders/workflows/custom_make/workflow.py index 208e9d469..14a06a5c4 100644 --- a/aws_lambda_builders/workflows/custom_make/workflow.py +++ b/aws_lambda_builders/workflows/custom_make/workflow.py @@ -2,7 +2,7 @@ ProvidedMakeWorkflow """ from aws_lambda_builders.workflows.custom_make.validator import CustomMakeRuntimeValidator -from aws_lambda_builders.workflow import BaseWorkflow, Capability, BuildInSourceSupport +from aws_lambda_builders.workflow import BaseWorkflow, Capability, BuildInSourceSupport, BuildDirectory from aws_lambda_builders.actions import CopySourceAction from aws_lambda_builders.path_resolver import PathResolver from .actions import CustomMakeAction @@ -23,7 +23,7 @@ class CustomMakeWorkflow(BaseWorkflow): EXCLUDED_FILES = (".aws-sam", ".git") - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): @@ -35,7 +35,6 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim self.os_utils = OSUtils() options = kwargs.get("options") or {} - build_in_source = kwargs.get("build_in_source") build_logical_id = options.get("build_logical_id", None) if not build_logical_id: @@ -48,9 +47,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim subprocess_make = SubProcessMake(make_exe=self.binaries["make"].binary_path, osutils=self.os_utils) # an explicitly definied working directory should take precedence - working_directory = options.get("working_directory") or self._select_working_directory( - source_dir, scratch_dir, build_in_source - ) + working_directory = options.get("working_directory") or self.build_dir make_action = CustomMakeAction( artifacts_dir, @@ -63,18 +60,12 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim self.actions = [] - if not self.build_in_source: - # if we're building on scratch_dir, we have to first copy the source there - self.actions.append(CopySourceAction(source_dir, scratch_dir, excludes=self.EXCLUDED_FILES)) + if working_directory != source_dir: + # if we're not building in the source directory, we have to first copy the source + self.actions.append(CopySourceAction(source_dir, working_directory, excludes=self.EXCLUDED_FILES)) self.actions.append(make_action) - def _select_working_directory(self, source_dir: str, scratch_dir: str, build_in_source: bool): - """ - Returns the directory where the make action should be executed - """ - return source_dir if build_in_source else scratch_dir - def get_resolvers(self): return [PathResolver(runtime="provided", binary="make", executable_search_paths=self.executable_search_paths)] diff --git a/aws_lambda_builders/workflows/dotnet_clipackage/workflow.py b/aws_lambda_builders/workflows/dotnet_clipackage/workflow.py index f514f31cf..320e7fd95 100644 --- a/aws_lambda_builders/workflows/dotnet_clipackage/workflow.py +++ b/aws_lambda_builders/workflows/dotnet_clipackage/workflow.py @@ -1,7 +1,7 @@ """ .NET Core CLI Package Workflow """ -from aws_lambda_builders.workflow import BaseWorkflow, Capability, BuildInSourceSupport +from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, Capability, BuildInSourceSupport from .actions import GlobalToolInstallAction, RunPackageAction from .dotnetcli import SubprocessDotnetCLI @@ -19,7 +19,7 @@ class DotnetCliPackageWorkflow(BaseWorkflow): CAPABILITY = Capability(language="dotnet", dependency_manager="cli-package", application_framework=None) - BUILD_IN_SOURCE_BY_DEFAULT = True + DEFAULT_BUILD_DIR = BuildDirectory.SOURCE BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.EXCLUSIVELY_SUPPORTED def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, mode=None, **kwargs): diff --git a/aws_lambda_builders/workflows/go_modules/workflow.py b/aws_lambda_builders/workflows/go_modules/workflow.py index 72ac03ba9..86f22a318 100644 --- a/aws_lambda_builders/workflows/go_modules/workflow.py +++ b/aws_lambda_builders/workflows/go_modules/workflow.py @@ -1,7 +1,7 @@ """ Go Modules Workflow """ -from aws_lambda_builders.workflow import BaseWorkflow, Capability, BuildInSourceSupport +from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, Capability, BuildInSourceSupport from .actions import GoModulesBuildAction from .builder import GoModulesBuilder @@ -15,7 +15,7 @@ class GoModulesWorkflow(BaseWorkflow): CAPABILITY = Capability(language="go", dependency_manager="modules", application_framework=None) - BUILD_IN_SOURCE_BY_DEFAULT = True + DEFAULT_BUILD_DIR = BuildDirectory.SOURCE BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.EXCLUSIVELY_SUPPORTED def __init__( diff --git a/aws_lambda_builders/workflows/java_gradle/workflow.py b/aws_lambda_builders/workflows/java_gradle/workflow.py index 9a2546dae..d83b54650 100644 --- a/aws_lambda_builders/workflows/java_gradle/workflow.py +++ b/aws_lambda_builders/workflows/java_gradle/workflow.py @@ -4,7 +4,7 @@ import hashlib import os from aws_lambda_builders.actions import CleanUpAction -from aws_lambda_builders.workflow import BaseWorkflow, Capability, BuildInSourceSupport +from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, Capability, BuildInSourceSupport from aws_lambda_builders.workflows.java.actions import JavaCopyDependenciesAction, JavaMoveDependenciesAction from aws_lambda_builders.workflows.java.utils import OSUtils @@ -25,7 +25,7 @@ class JavaGradleWorkflow(BaseWorkflow): INIT_FILE = "lambda-build-init.gradle" - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.NOT_SUPPORTED def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, **kwargs): diff --git a/aws_lambda_builders/workflows/java_maven/workflow.py b/aws_lambda_builders/workflows/java_maven/workflow.py index da80d17ae..164c365c8 100644 --- a/aws_lambda_builders/workflows/java_maven/workflow.py +++ b/aws_lambda_builders/workflows/java_maven/workflow.py @@ -1,7 +1,7 @@ """ Java Maven Workflow """ -from aws_lambda_builders.workflow import BaseWorkflow, Capability, BuildInSourceSupport +from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, Capability, BuildInSourceSupport from aws_lambda_builders.actions import CopySourceAction, CleanUpAction from aws_lambda_builders.workflows.java.actions import JavaCopyDependenciesAction, JavaMoveDependenciesAction from aws_lambda_builders.workflows.java.utils import OSUtils @@ -28,7 +28,7 @@ class JavaMavenWorkflow(BaseWorkflow): EXCLUDED_FILES = (".aws-sam", ".git") - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.NOT_SUPPORTED def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, **kwargs): diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 7f71fdf2f..70ebcc109 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -5,7 +5,7 @@ import logging from aws_lambda_builders.path_resolver import PathResolver -from aws_lambda_builders.workflow import BaseWorkflow, Capability, BuildInSourceSupport +from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, Capability, BuildInSourceSupport from aws_lambda_builders.actions import ( CopySourceAction, CleanUpAction, @@ -42,7 +42,7 @@ class NodejsNpmWorkflow(BaseWorkflow): CONFIG_PROPERTY = "aws_sam" - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.ARTIFACTS BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.NOT_SUPPORTED def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py index 1f6db83d8..0bc4c0ec5 100644 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py @@ -7,7 +7,7 @@ from pathlib import Path from typing import List -from aws_lambda_builders.workflow import BaseWorkflow, Capability, BuildInSourceSupport +from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, Capability, BuildInSourceSupport from aws_lambda_builders.actions import ( CopySourceAction, CleanUpAction, @@ -44,7 +44,7 @@ class NodejsNpmEsbuildWorkflow(BaseWorkflow): CONFIG_PROPERTY = "aws_sam" - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.NOT_SUPPORTED def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): diff --git a/aws_lambda_builders/workflows/python_pip/workflow.py b/aws_lambda_builders/workflows/python_pip/workflow.py index a304226be..58477cef4 100644 --- a/aws_lambda_builders/workflows/python_pip/workflow.py +++ b/aws_lambda_builders/workflows/python_pip/workflow.py @@ -3,7 +3,7 @@ """ import logging -from aws_lambda_builders.workflow import BaseWorkflow, BuildInSourceSupport, Capability +from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, BuildInSourceSupport, Capability from aws_lambda_builders.actions import CopySourceAction, CleanUpAction, LinkSourceAction from aws_lambda_builders.workflows.python_pip.validator import PythonRuntimeValidator from aws_lambda_builders.path_resolver import PathResolver @@ -67,7 +67,7 @@ class PythonPipWorkflow(BaseWorkflow): PYTHON_VERSION_THREE = "3" - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.NOT_SUPPORTED def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): diff --git a/aws_lambda_builders/workflows/ruby_bundler/workflow.py b/aws_lambda_builders/workflows/ruby_bundler/workflow.py index bc943affa..060664eec 100644 --- a/aws_lambda_builders/workflows/ruby_bundler/workflow.py +++ b/aws_lambda_builders/workflows/ruby_bundler/workflow.py @@ -3,7 +3,7 @@ """ import logging -from aws_lambda_builders.workflow import BaseWorkflow, BuildInSourceSupport, Capability +from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, BuildInSourceSupport, Capability from aws_lambda_builders.actions import CopySourceAction, CopyDependenciesAction, CleanUpAction from .actions import RubyBundlerInstallAction, RubyBundlerVendorAction from .utils import OSUtils @@ -25,7 +25,7 @@ class RubyBundlerWorkflow(BaseWorkflow): EXCLUDED_FILES = (".aws-sam", ".git") - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.ARTIFACTS BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.NOT_SUPPORTED def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): diff --git a/tests/functional/testdata/workflows/hello_workflow/write_hello.py b/tests/functional/testdata/workflows/hello_workflow/write_hello.py index 9987b1f5c..295480041 100644 --- a/tests/functional/testdata/workflows/hello_workflow/write_hello.py +++ b/tests/functional/testdata/workflows/hello_workflow/write_hello.py @@ -4,7 +4,7 @@ import os -from aws_lambda_builders.workflow import BaseWorkflow, BuildInSourceSupport, Capability +from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, BuildInSourceSupport, Capability from aws_lambda_builders.actions import BaseAction, Purpose @@ -34,7 +34,7 @@ class WriteHelloWorkflow(BaseWorkflow): NAME = "WriteHelloWorkflow" CAPABILITY = Capability(language="python", dependency_manager="test", application_framework="test") - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED def __init__(self, source_dir, artifacts_dir, *args, **kwargs): diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index ca99c058c..77bfb73ef 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -4,7 +4,7 @@ from parameterized import parameterized, param from aws_lambda_builders.builder import LambdaBuilder -from aws_lambda_builders.workflow import BuildInSourceSupport, Capability, BaseWorkflow +from aws_lambda_builders.workflow import BuildDirectory, BuildInSourceSupport, Capability, BaseWorkflow from aws_lambda_builders.registry import DEFAULT_REGISTRY @@ -71,7 +71,7 @@ class MyWorkflow(BaseWorkflow): CAPABILITY = Capability( language=self.lang, dependency_manager=self.lang_framework, application_framework=self.app_framework ) - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED def __init__( diff --git a/tests/unit/test_workflow.py b/tests/unit/test_workflow.py index 523ea6731..97dec9c8e 100644 --- a/tests/unit/test_workflow.py +++ b/tests/unit/test_workflow.py @@ -12,7 +12,7 @@ from aws_lambda_builders.binary_path import BinaryPath from aws_lambda_builders.validator import RuntimeValidator -from aws_lambda_builders.workflow import BaseWorkflow, BuildInSourceSupport, Capability +from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, BuildInSourceSupport, Capability from aws_lambda_builders.registry import get_workflow, DEFAULT_REGISTRY from aws_lambda_builders.exceptions import ( WorkflowFailedError, @@ -41,7 +41,7 @@ def test_must_register_one_workflow(self): class TestWorkflow(BaseWorkflow): NAME = "TestWorkflow" CAPABILITY = self.CAPABILITY1 - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED result_cls = get_workflow(self.CAPABILITY1) @@ -52,13 +52,13 @@ def test_must_register_two_workflows(self): class TestWorkflow1(BaseWorkflow): NAME = "TestWorkflow" CAPABILITY = self.CAPABILITY1 - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED class TestWorkflow2(BaseWorkflow): NAME = "TestWorkflow2" CAPABILITY = self.CAPABILITY2 - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED self.assertEqual(len(DEFAULT_REGISTRY), 2) @@ -71,7 +71,7 @@ def test_must_fail_if_name_not_present(self): class TestWorkflow1(BaseWorkflow): CAPABILITY = self.CAPABILITY1 - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED self.assertEqual(len(DEFAULT_REGISTRY), 0) @@ -83,7 +83,7 @@ def test_must_fail_if_capabilities_not_present(self): class TestWorkflow1(BaseWorkflow): NAME = "somename" - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED self.assertEqual(len(DEFAULT_REGISTRY), 0) @@ -96,7 +96,7 @@ def test_must_fail_if_capabilities_is_wrong_type(self): class TestWorkflow1(BaseWorkflow): NAME = "somename" CAPABILITY = "wrong data type" - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED self.assertEqual(len(DEFAULT_REGISTRY), 0) @@ -104,24 +104,40 @@ class TestWorkflow1(BaseWorkflow): @parameterized.expand( [ - (None, BuildInSourceSupport.NOT_SUPPORTED), # default not defined - (False, None), # support not defined - (False, BuildInSourceSupport.EXCLUSIVELY_SUPPORTED), # default incompatible with support - (True, False), # support not instance of enum + (None,), # support not defined + (False,), # support not instance of enum ] ) - def test_must_fail_if_build_in_source_variables_invalid(self, build_in_source_default, build_in_source_support): + def test_must_fail_if_build_in_source_support_invalid(self, build_in_source_support): with self.assertRaises(ValueError) as ctx: class TestWorkflow1(BaseWorkflow): NAME = "somename" CAPABILITY = self.CAPABILITY1 - BUILD_IN_SOURCE_BY_DEFAULT = build_in_source_default + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = build_in_source_support self.assertEqual(len(DEFAULT_REGISTRY), 0) + @parameterized.expand( + [ + (None,), # default build dir not defined + ("some_dir",), # default build dir not instance of enum + ] + ) + def test_must_fail_if_default_build_dir_invalid(self, default_build_dir): + + with self.assertRaises(ValueError) as ctx: + + class TestWorkflow1(BaseWorkflow): + NAME = "somename" + CAPABILITY = self.CAPABILITY1 + DEFAULT_BUILD_DIR = default_build_dir + BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.NOT_SUPPORTED + + self.assertEqual(len(DEFAULT_REGISTRY), 0) + class TestBaseWorkflow_init(TestCase): class MyWorkflow(BaseWorkflow): @@ -130,7 +146,7 @@ class MyWorkflow(BaseWorkflow): CAPABILITY = Capability( language="test", dependency_manager="testframework", application_framework="appframework" ) - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED def test_must_initialize_variables(self): @@ -143,7 +159,6 @@ def test_must_initialize_variables(self): executable_search_paths=[str(sys.executable)], optimizations={"a": "b"}, options={"c": "d"}, - build_in_source=True, ) self.assertEqual(self.work.source_dir, "source_dir") @@ -155,7 +170,6 @@ def test_must_initialize_variables(self): self.assertEqual(self.work.optimizations, {"a": "b"}) self.assertEqual(self.work.options, {"c": "d"}) self.assertEqual(self.work.architecture, "x86_64") - self.assertTrue(self.work.build_in_source) class TestBaseWorkflow_is_supported(TestCase): @@ -165,7 +179,7 @@ class MyWorkflow(BaseWorkflow): CAPABILITY = Capability( language="test", dependency_manager="testframework", application_framework="appframework" ) - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED def setUp(self): @@ -213,7 +227,7 @@ class MyWorkflow(BaseWorkflow): CAPABILITY = Capability( language="test", dependency_manager="testframework", application_framework="appframework" ) - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED def setUp(self): @@ -400,7 +414,7 @@ class MyWorkflow(BaseWorkflow): CAPABILITY = Capability( language="test", dependency_manager="testframework", application_framework="appframework" ) - BUILD_IN_SOURCE_BY_DEFAULT = False + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED def setUp(self): @@ -440,15 +454,49 @@ def test_must_pretty_print_workflow_info(self): class TestBaseWorkflow_build_in_source(TestCase): - @parameterized.expand([(True,), (False,)]) - def test_must_use_correct_default_value(self, default_value): + def test_must_use_source_directory_if_building_in_source(self): + class MyWorkflow(BaseWorkflow): + __TESTING__ = True + NAME = "MyWorkflow" + CAPABILITY = Capability( + language="test", dependency_manager="testframework", application_framework="appframework" + ) + DEFAULT_BUILD_DIR = BuildDirectory.SCRATCH + BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED + + source_dir = "source_dir" + + self.work = MyWorkflow( + source_dir, + "artifacts_dir", + "scratch_dir", + "manifest_path", + runtime="runtime", + executable_search_paths=[str(sys.executable)], + optimizations={"a": "b"}, + options={"c": "d"}, + build_in_source=True + ) + + self.assertEqual(self.work.build_dir, source_dir) + + @parameterized.expand( + [ + (BuildDirectory.SCRATCH, "scratch_dir"), + (BuildDirectory.SOURCE, "source_dir"), + (BuildDirectory.ARTIFACTS, "artifacts_dir"), + ] + ) + def test_must_use_correct_default_value( + self, default_build_dir, expected_build_dir + ): class MyWorkflow(BaseWorkflow): __TESTING__ = True NAME = "MyWorkflow" CAPABILITY = Capability( language="test", dependency_manager="testframework", application_framework="appframework" ) - BUILD_IN_SOURCE_BY_DEFAULT = default_value + DEFAULT_BUILD_DIR = default_build_dir BUILD_IN_SOURCE_SUPPORT = BuildInSourceSupport.OPTIONALLY_SUPPORTED self.work = MyWorkflow( @@ -462,21 +510,22 @@ class MyWorkflow(BaseWorkflow): options={"c": "d"}, ) - self.assertEqual(self.work.build_in_source, default_value) + self.assertEqual(self.work.build_dir, expected_build_dir) @parameterized.expand( [ - (True, False, BuildInSourceSupport.NOT_SUPPORTED), # want to build in source but it's not supported + (True, BuildInSourceSupport.NOT_SUPPORTED, BuildDirectory.SCRATCH, "scratch_dir"), # want to build in source but it's not supported ( False, - True, BuildInSourceSupport.EXCLUSIVELY_SUPPORTED, + BuildDirectory.SOURCE, + "source_dir" ), # don't want to build in source but workflow requires it - ("unsupported", False, BuildInSourceSupport.OPTIONALLY_SUPPORTED), # unsupported value passed in + ("unsupported", BuildInSourceSupport.OPTIONALLY_SUPPORTED, BuildDirectory.ARTIFACTS, "artifacts_dir"), # unsupported value passed in ] ) def test_must_use_default_if_unsupported_value_is_provided( - self, build_in_source_value, build_in_source_default, build_in_source_support + self, build_in_source_value, build_in_source_support, default_build_dir, expected_build_dir ): class MyWorkflow(BaseWorkflow): __TESTING__ = True @@ -484,7 +533,7 @@ class MyWorkflow(BaseWorkflow): CAPABILITY = Capability( language="test", dependency_manager="testframework", application_framework="appframework" ) - BUILD_IN_SOURCE_BY_DEFAULT = build_in_source_default + DEFAULT_BUILD_DIR = default_build_dir BUILD_IN_SOURCE_SUPPORT = build_in_source_support self.work = MyWorkflow( @@ -499,4 +548,4 @@ class MyWorkflow(BaseWorkflow): build_in_source=build_in_source_value, ) - self.assertEqual(self.work.build_in_source, build_in_source_default) + self.assertEqual(self.work.build_dir, expected_build_dir) diff --git a/tests/unit/workflows/custom_make/test_workflow.py b/tests/unit/workflows/custom_make/test_workflow.py index 028ef2945..2b89d59a1 100644 --- a/tests/unit/workflows/custom_make/test_workflow.py +++ b/tests/unit/workflows/custom_make/test_workflow.py @@ -93,6 +93,7 @@ def test_build_in_source_with_custom_working_directory(self): build_in_source=True, ) - self.assertEqual(len(workflow.actions), 1) - self.assertIsInstance(workflow.actions[0], CustomMakeAction) - self.assertEqual(workflow.actions[0].working_directory, working_dir) + self.assertEqual(len(workflow.actions), 2) + # since it's going to build in a custom working directory, should copy the source there first + self.assertIsInstance(workflow.actions[0], CopySourceAction) + self.assertEqual(workflow.actions[1].working_directory, working_dir) From b425e3893a3c4640b031da142df927570a6a4ba6 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Tue, 17 Jan 2023 16:20:48 -0800 Subject: [PATCH 2/6] typo --- aws_lambda_builders/workflow.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws_lambda_builders/workflow.py b/aws_lambda_builders/workflow.py index 9a1447759..9c7339a57 100644 --- a/aws_lambda_builders/workflow.py +++ b/aws_lambda_builders/workflow.py @@ -145,13 +145,13 @@ def __new__(mcs, name, bases, class_dict): if not isinstance(cls.CAPABILITY, Capability): raise ValueError("Workflow '{}' must register valid capabilities".format(cls.NAME)) - # All workflows define supported values for build in source + # All workflows must define supported values for build in source if not isinstance(cls.BUILD_IN_SOURCE_SUPPORT, BuildInSourceSupport): raise ValueError( "Workflow '{}' must define supported values for build in source".format(cls.NAME) ) - # All workflows define default build directory + # All workflows must define default build directory if not isinstance(cls.DEFAULT_BUILD_DIR, BuildDirectory): raise ValueError( "Workflow '{}' must define default build directory".format(cls.NAME) From 879d7d677dea931745d1fdc7037b5ae534c55b62 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Tue, 17 Jan 2023 16:40:23 -0800 Subject: [PATCH 3/6] black --- aws_lambda_builders/workflow.py | 13 +++++-------- tests/unit/test_workflow.py | 22 +++++++++++++++------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/aws_lambda_builders/workflow.py b/aws_lambda_builders/workflow.py index 9c7339a57..1013e997d 100644 --- a/aws_lambda_builders/workflow.py +++ b/aws_lambda_builders/workflow.py @@ -38,11 +38,13 @@ class BuildMode(object): DEBUG = "debug" RELEASE = "release" + class BuildDirectory(Enum): SCRATCH = "scratch" ARTIFACTS = "artifacts" SOURCE = "source" + class BuildInSourceSupport(Enum): """ Enum to define a workflow's support for building in source. @@ -147,15 +149,11 @@ def __new__(mcs, name, bases, class_dict): # All workflows must define supported values for build in source if not isinstance(cls.BUILD_IN_SOURCE_SUPPORT, BuildInSourceSupport): - raise ValueError( - "Workflow '{}' must define supported values for build in source".format(cls.NAME) - ) + raise ValueError("Workflow '{}' must define supported values for build in source".format(cls.NAME)) # All workflows must define default build directory if not isinstance(cls.DEFAULT_BUILD_DIR, BuildDirectory): - raise ValueError( - "Workflow '{}' must define default build directory".format(cls.NAME) - ) + raise ValueError("Workflow '{}' must define default build directory".format(cls.NAME)) LOG.debug("Registering workflow '%s' with capability '%s'", cls.NAME, cls.CAPABILITY) DEFAULT_REGISTRY[cls.CAPABILITY] = cls @@ -274,7 +272,6 @@ def __init__( self.actions = [] self._binaries = {} - def _select_build_dir(self, build_in_source: Optional[bool]) -> str: """ Returns the build directory for the workflow. @@ -284,7 +281,7 @@ def _select_build_dir(self, build_in_source: Optional[bool]) -> str: if build_in_source not in self.BUILD_IN_SOURCE_SUPPORT.value: # assign default value should_build_in_source = self.DEFAULT_BUILD_DIR == BuildDirectory.SOURCE - + # only show warning if an unsupported value was explicitly passed in if build_in_source is not None: LOG.warning( diff --git a/tests/unit/test_workflow.py b/tests/unit/test_workflow.py index 97dec9c8e..878387b6e 100644 --- a/tests/unit/test_workflow.py +++ b/tests/unit/test_workflow.py @@ -475,7 +475,7 @@ class MyWorkflow(BaseWorkflow): executable_search_paths=[str(sys.executable)], optimizations={"a": "b"}, options={"c": "d"}, - build_in_source=True + build_in_source=True, ) self.assertEqual(self.work.build_dir, source_dir) @@ -487,9 +487,7 @@ class MyWorkflow(BaseWorkflow): (BuildDirectory.ARTIFACTS, "artifacts_dir"), ] ) - def test_must_use_correct_default_value( - self, default_build_dir, expected_build_dir - ): + def test_must_use_correct_default_value(self, default_build_dir, expected_build_dir): class MyWorkflow(BaseWorkflow): __TESTING__ = True NAME = "MyWorkflow" @@ -514,14 +512,24 @@ class MyWorkflow(BaseWorkflow): @parameterized.expand( [ - (True, BuildInSourceSupport.NOT_SUPPORTED, BuildDirectory.SCRATCH, "scratch_dir"), # want to build in source but it's not supported + ( + True, + BuildInSourceSupport.NOT_SUPPORTED, + BuildDirectory.SCRATCH, + "scratch_dir", + ), # want to build in source but it's not supported ( False, BuildInSourceSupport.EXCLUSIVELY_SUPPORTED, BuildDirectory.SOURCE, - "source_dir" + "source_dir", ), # don't want to build in source but workflow requires it - ("unsupported", BuildInSourceSupport.OPTIONALLY_SUPPORTED, BuildDirectory.ARTIFACTS, "artifacts_dir"), # unsupported value passed in + ( + "unsupported", + BuildInSourceSupport.OPTIONALLY_SUPPORTED, + BuildDirectory.ARTIFACTS, + "artifacts_dir", + ), # unsupported value passed in ] ) def test_must_use_default_if_unsupported_value_is_provided( From 6f73545337165d9da3027b8d07bf0156a2cc95ef Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Wed, 18 Jan 2023 10:06:14 -0800 Subject: [PATCH 4/6] fix custom_make workflow to ensure it matches previous behaviour --- aws_lambda_builders/workflows/custom_make/workflow.py | 6 +++--- tests/unit/workflows/custom_make/test_workflow.py | 7 +++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/aws_lambda_builders/workflows/custom_make/workflow.py b/aws_lambda_builders/workflows/custom_make/workflow.py index 14a06a5c4..d5d6dbe23 100644 --- a/aws_lambda_builders/workflows/custom_make/workflow.py +++ b/aws_lambda_builders/workflows/custom_make/workflow.py @@ -46,7 +46,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim subprocess_make = SubProcessMake(make_exe=self.binaries["make"].binary_path, osutils=self.os_utils) - # an explicitly definied working directory should take precedence + # an explicitly defined working directory should take precedence working_directory = options.get("working_directory") or self.build_dir make_action = CustomMakeAction( @@ -60,9 +60,9 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim self.actions = [] - if working_directory != source_dir: + if self.build_dir != source_dir: # if we're not building in the source directory, we have to first copy the source - self.actions.append(CopySourceAction(source_dir, working_directory, excludes=self.EXCLUDED_FILES)) + self.actions.append(CopySourceAction(source_dir, self.build_dir, excludes=self.EXCLUDED_FILES)) self.actions.append(make_action) diff --git a/tests/unit/workflows/custom_make/test_workflow.py b/tests/unit/workflows/custom_make/test_workflow.py index 2b89d59a1..028ef2945 100644 --- a/tests/unit/workflows/custom_make/test_workflow.py +++ b/tests/unit/workflows/custom_make/test_workflow.py @@ -93,7 +93,6 @@ def test_build_in_source_with_custom_working_directory(self): build_in_source=True, ) - self.assertEqual(len(workflow.actions), 2) - # since it's going to build in a custom working directory, should copy the source there first - self.assertIsInstance(workflow.actions[0], CopySourceAction) - self.assertEqual(workflow.actions[1].working_directory, working_dir) + self.assertEqual(len(workflow.actions), 1) + self.assertIsInstance(workflow.actions[0], CustomMakeAction) + self.assertEqual(workflow.actions[0].working_directory, working_dir) From a5902a0bb1612e47f7562da21fee941ea6fcc921 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Wed, 18 Jan 2023 11:31:17 -0800 Subject: [PATCH 5/6] add comment --- aws_lambda_builders/workflow.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/aws_lambda_builders/workflow.py b/aws_lambda_builders/workflow.py index 1013e997d..74a28c5e1 100644 --- a/aws_lambda_builders/workflow.py +++ b/aws_lambda_builders/workflow.py @@ -266,6 +266,8 @@ def __init__( self.architecture = architecture self.is_building_layer = is_building_layer self.experimental_flags = experimental_flags if experimental_flags else [] + + # this represents where the build/install happens, not the final output directory (that's the artifacts_dir) self.build_dir = self._select_build_dir(build_in_source) # Actions are registered by the subclasses as they seem fit From 7132cb640c5913eb06a76b16fc93a4da2ab6d7b1 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Wed, 18 Jan 2023 11:53:17 -0800 Subject: [PATCH 6/6] update comment --- aws_lambda_builders/workflow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws_lambda_builders/workflow.py b/aws_lambda_builders/workflow.py index 74a28c5e1..3a861aa4a 100644 --- a/aws_lambda_builders/workflow.py +++ b/aws_lambda_builders/workflow.py @@ -182,7 +182,7 @@ class BaseWorkflow(object, metaclass=_WorkflowMetaClass): # Support for building in source, each workflow should define this. BUILD_IN_SOURCE_SUPPORT = None - # Default build directory, each workflow should define this. + # The directory where the workflow builds/installs by default, each workflow should define this. DEFAULT_BUILD_DIR = None def __init__(