From 47a6d2d57c7eebb166b5d77df61cd80910dc00c7 Mon Sep 17 00:00:00 2001 From: Carolina Lopes <116589288+ccruzagralopes@users.noreply.github.com> Date: Thu, 25 Jan 2024 09:36:37 -0300 Subject: [PATCH] Create collections on init files (#60) * Create collections on inits * Update python tests submodule * Apply code review suggestion * Fix unit test * Fix spelling errors --- app/container_manager/container_manager.py | 2 +- app/test_engine/test_collection_discovery.py | 140 ++++++++++++------ app/tests/conftest.py | 2 +- .../test_engine/test_collection_discovery.py | 10 +- cspell.json | 1 + test_collections/python_tests | 2 +- test_collections/sample_tests/__init__.py | 22 +++ .../tool_blocklist_unit_tests/__init__.py | 24 +++ test_collections/tool_unit_tests/__init__.py | 24 +++ 9 files changed, 172 insertions(+), 55 deletions(-) diff --git a/app/container_manager/container_manager.py b/app/container_manager/container_manager.py index e5af9206..c7bd7397 100644 --- a/app/container_manager/container_manager.py +++ b/app/container_manager/container_manager.py @@ -93,7 +93,7 @@ def __run_new_container(self, docker_image_tag: str, parameters: Dict) -> Contai return self.__client.containers.run(docker_image_tag, **parameters) except DockerException as error: logger.error( - "Error ocurred while creating a container from image " + str(error) + "Error occurred while creating a container from image " + str(error) ) raise error diff --git a/app/test_engine/test_collection_discovery.py b/app/test_engine/test_collection_discovery.py index 3854463c..30689233 100644 --- a/app/test_engine/test_collection_discovery.py +++ b/app/test_engine/test_collection_discovery.py @@ -42,7 +42,7 @@ DISABLED_TEST_CASES_FILEPATH = COLLECTIONS_PATH / DISABLED_TEST_CASES_FILENAME -def disabled_test_collections() -> Optional[List[str]]: +def disabled_test_collections() -> List[str]: """Returns a list of collection names that should be disabled. Each line in the file at DISABLED_COLLECTIONS_FILEPATH corresponds to a @@ -57,7 +57,7 @@ def disabled_test_collections() -> Optional[List[str]]: return __extract_lines_from_file(DISABLED_COLLECTIONS_FILEPATH) -def disabled_test_cases() -> Optional[List[str]]: +def disabled_test_cases() -> List[str]: """Returns a list of public ids from test cases that should be disabled. Each line in the file at DISABLED_TEST_CASES_FILEPATH corresponds to a test case @@ -72,7 +72,7 @@ def disabled_test_cases() -> Optional[List[str]]: return __extract_lines_from_file(DISABLED_TEST_CASES_FILEPATH) -def __extract_lines_from_file(file_path: Path) -> Optional[List[str]]: +def __extract_lines_from_file(file_path: Path) -> List[str]: """Returns a list of strings extracted from a file. Each line in the file corresponds to a item in the list. @@ -82,7 +82,7 @@ def __extract_lines_from_file(file_path: Path) -> Optional[List[str]]: """ if not file_path.exists(): logger.warning(f"No file found at #{file_path}") - return None + return [] # Load the config from file as a dictionary. with open(file_path) as file: @@ -90,8 +90,8 @@ def __extract_lines_from_file(file_path: Path) -> Optional[List[str]]: def discover_test_collections( - disabled_collections: Optional[list[str]] = disabled_test_collections(), - disabled_test_cases: Optional[list[str]] = disabled_test_cases(), + disabled_collections: list[str] = disabled_test_collections(), + disabled_test_cases: list[str] = disabled_test_cases(), ) -> Dict[str, TestCollectionDeclaration]: """Dynamically discover test_collection modules in `test_collections` folder. @@ -110,15 +110,12 @@ def discover_test_collections( names = __test_collection_folder_names() - # Apply filter if needed - if disabled_collections: - names = [n for n in names if n not in disabled_collections] - for name in names: # Don't add collection if it doesn't have any suites if found_collections := __find_test_collections(name, disabled_test_cases): for collection in found_collections: - collections[collection.name] = collection + if collection.name not in disabled_collections: + collections[collection.name] = collection return collections @@ -201,48 +198,57 @@ def __find_test_collections( - TestSuites cannot be nested, i.e. one root TestSuite for a set of test cases. - TestCases are nested within a TestSuite. """ - collection_path = f"{COLLECTIONS_PATH}/{folder_name}" collection_module_name = f"{COLLECTIONS_DIRNAME}.{folder_name}" - # If Collection defines a TestCollectionDeclarations return them - if test_collections := __declared_collection_declarations(collection_module_name): - if disabled_test_cases: - # Remove disabled test cases from collections - for collection in test_collections: - for _, suite_decl in collection.test_suites.items(): - suite_decl.test_cases = { - k: v - for k, v in suite_decl.test_cases.items() - if v.public_id not in disabled_test_cases - } - return test_collections - - # Return value place holder - test_collection = TestCollectionDeclaration(path=collection_path, name=folder_name) - - test_suites: List[Type[TestSuite]] = __find_classes_of_type( - module_name=collection_module_name, classtype=TestSuite - ) + test_collections = __declared_collection_declarations(collection_module_name) - for suite in test_suites: - suite_declaration = __find_test_suite( - suite=suite, - disabled_test_cases=disabled_test_cases, - ) + # Remove disabled test cases from collections + if disabled_test_cases: + __remove_disabled_test_cases(test_collections, disabled_test_cases) - if suite_declaration: - test_collection.test_suites[suite.public_id()] = suite_declaration + return test_collections - # Don't include empty test collections - if not test_collection.test_suites: - return None - return [test_collection] +def __remove_disabled_test_cases( + test_collections: list[TestCollectionDeclaration], + disabled_test_cases: list[str], +) -> None: + """Remove the disabled test cases from a list of test collections. + + If all test cases from a test suite are disabled, the test suite is removed from the + test collection. If all test suites from a test collection are removed, then the + collection itself is also removed form the list. + + Args: + test_collections (list[TestCollectionDeclaration]): The test collections list. + disabled_test_cases (list[str]): The list of the disabled test cases. + """ + emptied_collections = [] + + for index, collection in enumerate(test_collections): + emptied_suites = [] + + for key, suite_decl in collection.test_suites.items(): + suite_decl.test_cases = { + k: v + for k, v in suite_decl.test_cases.items() + if v.public_id not in disabled_test_cases + } + if not suite_decl.test_cases: + emptied_suites.append(key) + + for emptied_suite in emptied_suites: + del collection.test_suites[emptied_suite] + + if not collection.test_suites: + emptied_collections.append(index) + + for emptied_collection in emptied_collections: + del test_collections[emptied_collection] def __find_test_suite( suite: Type[TestSuite], - disabled_test_cases: Optional[list[str]], ) -> Optional[TestSuiteDeclaration]: """Dynamically finds TestCases in the specified suite. @@ -255,10 +261,6 @@ def __find_test_suite( module_name=suite.__module__, classtype=TestCase ) - # Apply filter if needed - if disabled_test_cases: - test_cases = [x for x in test_cases if x.public_id() not in disabled_test_cases] - # Don't include empty test suites if not test_cases: return None @@ -271,6 +273,50 @@ def __find_test_suite( return suite_declaration +def test_collection_declaration( + collection_path: Path, name: str +) -> Optional[TestCollectionDeclaration]: + """Declare a new collection of test suites.""" + collection = TestCollectionDeclaration(path=str(collection_path), name=name) + + collection_module_name = __collection_module_name(collection_path) + suite_types = __find_classes_of_type( + module_name=collection_module_name, classtype=TestSuite + ) + + for suite_type in suite_types: + if suite := __find_test_suite(suite_type): + collection.add_test_suite(suite) + + return collection if collection.test_suites else None + + +def __collection_module_name(path: Path) -> str: + """Get the name of the module that should be searched for the collection's test + suites. + + The module name is extracted from the collection folder path. This function iterates + over the parts of the path until it finds COLLECTIONS_DIRNAME. Then it creates the + module name string with the remaining parts. + + Args: + path (Path): Collection folder path. + + Returns: + str: Collection module name starting w + """ + found_collections_directory = False + collection_module_name = COLLECTIONS_DIRNAME + + for part in path.parts: + if found_collections_directory: + collection_module_name += f".{part}" + elif part == COLLECTIONS_DIRNAME: + found_collections_directory = True + + return collection_module_name + + def __test_collection_folder_names() -> List[str]: """This will return all folder names for sub-folder in the test collections folder. diff --git a/app/tests/conftest.py b/app/tests/conftest.py index b5bbef21..bbefcd94 100644 --- a/app/tests/conftest.py +++ b/app/tests/conftest.py @@ -121,5 +121,5 @@ def block_on_serial_marker(request: pytest.FixtureRequest) -> Generator: unit tests. Make sure we discover all test collections here. """ test_script_manager.test_script_manager.test_collections = discover_test_collections( - disabled_collections=None + disabled_collections=[] ) diff --git a/app/tests/test_engine/test_collection_discovery.py b/app/tests/test_engine/test_collection_discovery.py index ec34ba06..947ea934 100644 --- a/app/tests/test_engine/test_collection_discovery.py +++ b/app/tests/test_engine/test_collection_discovery.py @@ -38,20 +38,20 @@ def test_extract_lines_from_file_with_existing_file_returns_lines() -> None: assert result == file_lines -def test_extract_lines_from_file_with_nonexisting_file_returns_none() -> None: +def test_extract_lines_from_file_with_nonexisting_file_returns_empty() -> None: directory_path = Path(__file__).parent new_file_path = directory_path / f"tempFile_{uuid1()}" result = __extract_lines_from_file(new_file_path) - assert result is None + assert result == [] def test_discover_test_collections_disable_one_test_case() -> None: disabled_test_cases = ["TC_Blocklist_2_1"] collections = discover_test_collections( - disabled_collections=None, disabled_test_cases=disabled_test_cases + disabled_collections=[], disabled_test_cases=disabled_test_cases ) test_collection_names = [] @@ -76,7 +76,7 @@ def test_discover_test_collections_disable_all_test_cases_in_suite() -> None: disabled_test_cases = ["TC_Blocklist_2_1", "TC_Blocklist_2_2"] collections = discover_test_collections( - disabled_collections=None, disabled_test_cases=disabled_test_cases + disabled_collections=[], disabled_test_cases=disabled_test_cases ) test_collection_names = [] @@ -101,7 +101,7 @@ def test_discover_test_collections_disable_all_test_cases_in_collection() -> Non disabled_test_cases = ["TC_Blocklist_1_1", "TC_Blocklist_2_1", "TC_Blocklist_2_2"] collections = discover_test_collections( - disabled_collections=None, disabled_test_cases=disabled_test_cases + disabled_collections=[], disabled_test_cases=disabled_test_cases ) test_collection_names = [] diff --git a/cspell.json b/cspell.json index 5963d04b..022f992a 100644 --- a/cspell.json +++ b/cspell.json @@ -66,6 +66,7 @@ "asgi", "c0ffee", "cond", + "DGGEN", "gunicorn", "httpx", "nodeid", diff --git a/test_collections/python_tests b/test_collections/python_tests index 5367de32..8b20f355 160000 --- a/test_collections/python_tests +++ b/test_collections/python_tests @@ -1 +1 @@ -Subproject commit 5367de326ceefc4cf1b4f60a34ee4610e11eb336 +Subproject commit 8b20f355c52a0bb9a154ef79e9859cac5546fbb1 diff --git a/test_collections/sample_tests/__init__.py b/test_collections/sample_tests/__init__.py index e69de29b..11147391 100644 --- a/test_collections/sample_tests/__init__.py +++ b/test_collections/sample_tests/__init__.py @@ -0,0 +1,22 @@ +# +# Copyright (c) 2024 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +from pathlib import Path + +from app.test_engine.test_collection_discovery import test_collection_declaration + +COLLECTION_PATH = Path(__file__).parent + +sample_tests_collection = test_collection_declaration(COLLECTION_PATH, "sample_tests") diff --git a/test_collections/tool_blocklist_unit_tests/__init__.py b/test_collections/tool_blocklist_unit_tests/__init__.py index e69de29b..bdf82762 100644 --- a/test_collections/tool_blocklist_unit_tests/__init__.py +++ b/test_collections/tool_blocklist_unit_tests/__init__.py @@ -0,0 +1,24 @@ +# +# Copyright (c) 2024 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +from pathlib import Path + +from app.test_engine.test_collection_discovery import test_collection_declaration + +COLLECTION_PATH = Path(__file__).parent + +tool_blocklist_unit_tests_collection = test_collection_declaration( + COLLECTION_PATH, "tool_blocklist_unit_tests" +) diff --git a/test_collections/tool_unit_tests/__init__.py b/test_collections/tool_unit_tests/__init__.py index e69de29b..11e3688d 100644 --- a/test_collections/tool_unit_tests/__init__.py +++ b/test_collections/tool_unit_tests/__init__.py @@ -0,0 +1,24 @@ +# +# Copyright (c) 2024 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +from pathlib import Path + +from app.test_engine.test_collection_discovery import test_collection_declaration + +COLLECTION_PATH = Path(__file__).parent + +tool_unit_tests_collection = test_collection_declaration( + COLLECTION_PATH, "tool_unit_tests" +)