Apply suggestions from code review (1)

* `_exclude_files_from_build` becomes a static method
* Replace ternary expression with simple  `if/else` statement

Co-Authored-By: Graham Hammond <[email protected]>

Author: hugueskamba

tools/test/build_api/build_api_test.py

@@ -20,7 +20,7 @@
 from mock import patch, MagicMock
 from tools.build_api import prepare_toolchain, build_project, build_library
 from tools.regions import merge_region_list
-from tools.resources import Resources
+from tools.resources import Resources, FileType
 from tools.toolchains import TOOLCHAINS
 from tools.notifier.mock import MockNotifier
 from tools.config import Region, Config, ConfigException
@@ -108,14 +108,7 @@ def test_compile_legacy_sources_always_complete_build(self, *_):
         assert any('percent' in msg and msg['percent'] == 100.0
                    for msg in notify.messages if msg)
 
-    @patch('tools.toolchains.arm.ARM_STD.parse_dependencies',
-           return_value=["foo"])
-    @patch('tools.toolchains.mbedToolchain.need_update',
-           side_effect=[i % 2 for i in range(3000)])
-    @patch('os.mkdir')
-    @patch('tools.toolchains.mbedToolchain.dump_build_profile')
-    @patch('tools.utils.run_cmd', return_value=(b'', b'', 0))
-    def test_dirs_exclusion_from_build(self, *_):
+    def test_dirs_exclusion_from_file_to_compile(self, *_):
         """Test that dirs can be excluded from the build."""
         notify = MockNotifier()
         toolchain = prepare_toolchain(self.src_paths, self.build_path, self.target,
@@ -127,19 +120,26 @@ def test_dirs_exclusion_from_build(self, *_):
         toolchain.RESPONSE_FILES=False
         toolchain.config_processed = True
         toolchain.config_file = "junk"
-        exclude_dirs = ['platform/','rtos/', 'targets/']
+        exclude_dirs = ['platform/','drivers/', 'targets/']
+        exclude_dirs2 = []
 
-        toolchain.compile_legacy_sources(
-            res, inc_dirs=None, exclude_dirs = exclude_dirs
+        files_to_compile = (
+            res.get_file_refs(FileType.ASM_SRC) +
+            res.get_file_refs(FileType.C_SRC) +
+            res.get_file_refs(FileType.CPP_SRC)
+        )
+        compilation_queue = toolchain._exclude_files_from_build(
+            files_to_compile, exclude_dirs
         )
+        print(len(compilation_queue))
+
+        assert(len(compilation_queue) < len(files_to_compile))
         assert all(
-            exclude_dir not in msg
-            for msg in notify.messages
-            if msg
+            exclude_dir not in file_to_compile.path
+            for file_to_compile in compilation_queue
             for exclude_dir in exclude_dirs
         )
 
-
     @patch('tools.build_api.Config')
     def test_prepare_toolchain_app_config(self, mock_config_init):
         """

tools/toolchains/mbed_toolchain.py

@@ -413,7 +413,8 @@ def compile_sources(self, resources, inc_dirs=None):
         """Compile source files."""
         return self._compile_sources(resources, inc_dirs=inc_dirs)
 
-    def _exclude_files_from_build(self, files_to_compile, exclude_dirs):
+    @staticmethod
+    def _exclude_files_from_build(files_to_compile, exclude_dirs):
         """Remove files from dirs to be excluded for the build."""
         return [
             file_to_compile
@@ -431,11 +432,11 @@ def _compile_sources(self, resources, inc_dirs=None, exclude_dirs=None):
             resources.get_file_refs(FileType.C_SRC) +
             resources.get_file_refs(FileType.CPP_SRC)
         )
-        compilation_queue = (
-            files_to_compile
-            if not exclude_dirs
-            else self._exclude_files_from_build(files_to_compile, exclude_dirs)
-        )
+        if exclude_dirs:
+            compilation_queue = self._exclude_files_from_build(files_to_compile, exclude_dirs)
+        else:
+            compilation_queue = files_to_compile
+
         self.to_be_compiled = len(compilation_queue)
         self.compiled = 0