From 4b864900d1417a0a7b306253029355e853126af6 Mon Sep 17 00:00:00 2001 From: JWM Date: Wed, 14 Aug 2024 15:21:53 +0200 Subject: [PATCH 01/48] Change `list` to more descriptive name `values` --- mlx/coverity_services.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index 3974ee67..a5b6ea25 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -300,7 +300,7 @@ def get_defects(self, stream, filters, column_names): } ] - Filter = namedtuple("Filter", "name matcher_type list allow_regex", defaults=[None, False]) + Filter = namedtuple("Filter", "name matcher_type values allow_regex", defaults=[None, False]) filter_options = { "checker": Filter("Checker", "keyMatcher", self.checkers, True), "impact": Filter("Impact", "keyMatcher", IMPACT_LIST), @@ -314,7 +314,7 @@ def get_defects(self, stream, filters, column_names): for option, filter in filter_options.items(): if filters[option]: filter_values = self.handle_attribute_filter( - filters[option], filter.name, filter.list, filter.allow_regex + filters[option], filter.name, filter.values, filter.allow_regex ) if filter_values: query_filters.append(self.assemble_query_filter(filter.name, filter_values, filter.matcher_type)) From 2c012cde5a99ceba0d87196f87a0514f97ccba74 Mon Sep 17 00:00:00 2001 From: JWM Date: Wed, 14 Aug 2024 15:31:47 +0200 Subject: [PATCH 02/48] Use function `validate_filter_option` directly in `handle_attribute_filter` --- mlx/coverity_services.py | 51 ++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 33 deletions(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index a5b6ea25..647a60b2 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -206,36 +206,6 @@ def _request(self, url, data=None): self.logger.warning(err_msg) return response.raise_for_status() - @staticmethod - def validate_filter_option(name, req_csv, valid_attributes, allow_regex=False): - """Add filter when the attribute is valid. If `valid_attributes` is empty or falsy, - all attributes of the CSV list are valid. - The CSV list can allow regular expressions when `allow_regex` is set to True. - - Args: - name (str): String representation of the attribute. - req_csv (str): A CSV list of attribute values to query. - valid_attributes (list/dict): The valid attributes. - allow_regex (bool): True to treat filter values as regular expressions, False to require exact matches - - Returns: - list[str]: The list of valid attributes - """ - logging.info("Validate required %s [%s]", name, req_csv) - filter_values = [] - for field in req_csv.split(","): - if not valid_attributes or field in valid_attributes: - logging.info("Classification [%s] is valid", field) - filter_values.append(field) - elif allow_regex: - pattern = re.compile(field) - for element in valid_attributes: - if pattern.search(element) and element not in filter_values: - filter_values.append(element) - else: - logging.error("Invalid %s filter: %s", name, field) - return filter_values - def assemble_query_filter(self, column_name, filter_values, matcher_type): """Assemble a filter for a specific column @@ -342,18 +312,33 @@ def get_defects(self, stream, filters, column_names): logging.info("Running Coverity query...") return self.retrieve_issues(data) - def handle_attribute_filter(self, attribute_values, name, *args, **kwargs): - """Applies any filter on an attribute's values. + def handle_attribute_filter(self, attribute_values, name, valid_attributes, allow_regex=False): + """Add filter when the attribute is valid. If `valid_attributes` is empty or falsy, + all attributes of the CSV list are valid. + The CSV list can allow regular expressions when `allow_regex` is set to True. Args: attribute_values (str): A CSV list of attribute values to query. name (str): String representation of the attribute. + valid_attributes (list/dict): The valid attributes. + allow_regex (bool): True to treat filter values as regular expressions, False to require exact matches Returns: list[str]: The list of valid attributes """ logging.info("Using %s filter [%s]", name, attribute_values) - filter_values = self.validate_filter_option(name, attribute_values, *args, **kwargs) + filter_values = [] + for field in attribute_values.split(","): + if not valid_attributes or field in valid_attributes: + logging.info("Classification [%s] is valid", field) + filter_values.append(field) + elif allow_regex: + pattern = re.compile(field) + for element in valid_attributes: + if pattern.search(element) and element not in filter_values: + filter_values.append(element) + else: + logging.error("Invalid %s filter: %s", name, field) return filter_values def handle_component_filter(self, attribute_values): From 1ab9fa00e69b63cd8465272bf40285fac06d8144 Mon Sep 17 00:00:00 2001 From: JWM Date: Wed, 14 Aug 2024 15:39:15 +0200 Subject: [PATCH 03/48] Use empty list instead of None to avoid error --- mlx/coverity_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index 647a60b2..96d86c19 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -270,7 +270,7 @@ def get_defects(self, stream, filters, column_names): } ] - Filter = namedtuple("Filter", "name matcher_type values allow_regex", defaults=[None, False]) + Filter = namedtuple("Filter", "name matcher_type values allow_regex", defaults=[[], False]) filter_options = { "checker": Filter("Checker", "keyMatcher", self.checkers, True), "impact": Filter("Impact", "keyMatcher", IMPACT_LIST), From db357d59c14d2fe8ec644fa8e55117598ef89a7a Mon Sep 17 00:00:00 2001 From: JWM Date: Wed, 14 Aug 2024 15:40:15 +0200 Subject: [PATCH 04/48] Update docstring --- mlx/coverity_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index 96d86c19..154d6224 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -313,7 +313,7 @@ def get_defects(self, stream, filters, column_names): return self.retrieve_issues(data) def handle_attribute_filter(self, attribute_values, name, valid_attributes, allow_regex=False): - """Add filter when the attribute is valid. If `valid_attributes` is empty or falsy, + """Add filter when the attribute is valid. If `valid_attributes` is empty, all attributes of the CSV list are valid. The CSV list can allow regular expressions when `allow_regex` is set to True. From b48d82a16245b83b60feafddaecca8c0ad0d191e Mon Sep 17 00:00:00 2001 From: JWM Date: Wed, 14 Aug 2024 15:45:19 +0200 Subject: [PATCH 05/48] Use set to delete checking uniqueness in if statement --- mlx/coverity_services.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index 154d6224..8241758c 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -327,16 +327,16 @@ def handle_attribute_filter(self, attribute_values, name, valid_attributes, allo list[str]: The list of valid attributes """ logging.info("Using %s filter [%s]", name, attribute_values) - filter_values = [] + filter_values = set() for field in attribute_values.split(","): if not valid_attributes or field in valid_attributes: logging.info("Classification [%s] is valid", field) - filter_values.append(field) + filter_values.add(field) elif allow_regex: pattern = re.compile(field) for element in valid_attributes: - if pattern.search(element) and element not in filter_values: - filter_values.append(element) + if pattern.search(element): + filter_values.add(element) else: logging.error("Invalid %s filter: %s", name, field) return filter_values From 3de603f738cbdea0e04d31c79f99bcb3a2de60d9 Mon Sep 17 00:00:00 2001 From: JWM Date: Fri, 16 Aug 2024 10:40:15 +0200 Subject: [PATCH 06/48] Remove Python version 3.7; Add Python 3.12 --- .github/workflows/python-package.yml | 2 +- setup.py | 2 +- tox.ini | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index a393811b..64cf0adb 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -14,7 +14,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"] + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] steps: - uses: actions/checkout@v3 - name: Set up Python ${{ matrix.python-version }} diff --git a/setup.py b/setup.py index 5d67d035..ad241ef5 100644 --- a/setup.py +++ b/setup.py @@ -29,11 +29,11 @@ 'License :: OSI Approved :: GNU General Public License v3 (GPLv3)', 'Operating System :: OS Independent', 'Programming Language :: Python', - 'Programming Language :: Python :: 3.7', 'Programming Language :: Python :: 3.8', 'Programming Language :: Python :: 3.9', 'Programming Language :: Python :: 3.10', 'Programming Language :: Python :: 3.11', + 'Programming Language :: Python :: 3.12', 'Topic :: Documentation', 'Topic :: Documentation :: Sphinx', 'Topic :: Utilities', diff --git a/tox.ini b/tox.ini index 10fd59b7..a48d3098 100644 --- a/tox.ini +++ b/tox.ini @@ -1,26 +1,26 @@ [tox] envlist = - py37, py38, py39, py310, py311 + py38, py39, py310, py311, py312 clean, check, [gh-actions] python = - 3.7: py37 3.8: py38 3.9: py39 3.10: py310 3.11: py311 + 3.12: py312 [testenv] basepython = py: python3 pypy: {env:TOXPYTHON:pypy} - py37: {env:TOXPYTHON:python3.7} py38: {env:TOXPYTHON:python3.8} py39: {env:TOXPYTHON:python3.9} py310: {env:TOXPYTHON:python3.10} py311: {env:TOXPYTHON:python3.11} + py312: {env:TOXPYTHON:python3.12} {clean,test,html,latexpdf,check,report,coverage}: python3 setenv = PYTHONPATH={toxinidir}/tests From 8232fb63bcf314b3efed992c367c27f3d06aeeb9 Mon Sep 17 00:00:00 2001 From: JWM Date: Fri, 16 Aug 2024 10:49:11 +0200 Subject: [PATCH 07/48] Add warning to exclude that is not resolved yet in the sphinx traceability extension --- warnings_config.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/warnings_config.json b/warnings_config.json index e59637db..888ab4b0 100644 --- a/warnings_config.json +++ b/warnings_config.json @@ -6,7 +6,8 @@ "exclude": [ "WARNING: Connection failed: ", "WARNING: Connection failed: ", - "CRITICAL:root:No such Coverity Configuration Service" + "CRITICAL:root:No such Coverity Configuration Service", + "WARNING: cannot cache unpickable configuration value: 'traceability_attributes_sort' \\(because it contains a function, class, or module object\\)" ] } } From d0957686e9f530f1a724e1d734a2d32b330a40a0 Mon Sep 17 00:00:00 2001 From: JWM <62558419+JokeWaumans@users.noreply.github.com> Date: Fri, 16 Aug 2024 10:54:11 +0200 Subject: [PATCH 08/48] Update docstring Co-authored-by: Jasper Craeghs <28319872+JasperCraeghs@users.noreply.github.com> --- mlx/coverity_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index 8241758c..80a37a73 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -324,7 +324,7 @@ def handle_attribute_filter(self, attribute_values, name, valid_attributes, allo allow_regex (bool): True to treat filter values as regular expressions, False to require exact matches Returns: - list[str]: The list of valid attributes + set[str]: The attributes values to query with """ logging.info("Using %s filter [%s]", name, attribute_values) filter_values = set() From a6f0fb6a2b24912d15b58b739a770cace71134e3 Mon Sep 17 00:00:00 2001 From: JWM <62558419+JokeWaumans@users.noreply.github.com> Date: Fri, 16 Aug 2024 10:54:24 +0200 Subject: [PATCH 09/48] Update docstring Co-authored-by: Jasper Craeghs <28319872+JasperCraeghs@users.noreply.github.com> --- mlx/coverity_services.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index 80a37a73..4b628971 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -313,8 +313,7 @@ def get_defects(self, stream, filters, column_names): return self.retrieve_issues(data) def handle_attribute_filter(self, attribute_values, name, valid_attributes, allow_regex=False): - """Add filter when the attribute is valid. If `valid_attributes` is empty, - all attributes of the CSV list are valid. + """Process the given CSV list of attribute values by filtering out the invalid ones while logging an error. The CSV list can allow regular expressions when `allow_regex` is set to True. Args: From d7e4675bcba2f7f80df4558aed74c1033d468ab6 Mon Sep 17 00:00:00 2001 From: JWM <62558419+JokeWaumans@users.noreply.github.com> Date: Fri, 16 Aug 2024 10:54:43 +0200 Subject: [PATCH 10/48] Update docstring Co-authored-by: Jasper Craeghs <28319872+JasperCraeghs@users.noreply.github.com> --- mlx/coverity_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index 4b628971..efe36b4b 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -319,7 +319,7 @@ def handle_attribute_filter(self, attribute_values, name, valid_attributes, allo Args: attribute_values (str): A CSV list of attribute values to query. name (str): String representation of the attribute. - valid_attributes (list/dict): The valid attributes. + valid_attributes (list/dict): All valid/possible attribute values. allow_regex (bool): True to treat filter values as regular expressions, False to require exact matches Returns: From a3a56c8808b4743f31221c053995c870721751eb Mon Sep 17 00:00:00 2001 From: JWM Date: Fri, 16 Aug 2024 10:57:46 +0200 Subject: [PATCH 11/48] Add python_requires='>=3.8' to setup.py --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index ad241ef5..76b93458 100644 --- a/setup.py +++ b/setup.py @@ -42,6 +42,7 @@ packages=find_packages(exclude=['tests', 'example']), include_package_data=True, install_requires=requires, + python_requires='>=3.8', namespace_packages=['mlx'], keywords=[ 'coverity', From 49c25806018c7b84aa634516f635f7b5f208627f Mon Sep 17 00:00:00 2001 From: JWM <62558419+JokeWaumans@users.noreply.github.com> Date: Fri, 16 Aug 2024 11:21:32 +0200 Subject: [PATCH 12/48] Use the new walrus operator Co-authored-by: Jasper Craeghs <28319872+JasperCraeghs@users.noreply.github.com> --- mlx/coverity_services.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index efe36b4b..35730498 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -289,10 +289,8 @@ def get_defects(self, stream, filters, column_names): if filter_values: query_filters.append(self.assemble_query_filter(filter.name, filter_values, filter.matcher_type)) - if filters["component"]: - filter_values = self.handle_component_filter(filters["component"]) - if filter_values: - query_filters.append(self.assemble_query_filter("Component", filter_values, "nameMatcher")) + if filter := filters["component"] and filter_values := self.handle_component_filter(filter): + query_filters.append(self.assemble_query_filter("Component", filter_values, "nameMatcher")) data = { "filters": query_filters, From e31b7166e3f48603aaf680a248a9bef91bc97466 Mon Sep 17 00:00:00 2001 From: JWM Date: Fri, 16 Aug 2024 11:31:31 +0200 Subject: [PATCH 13/48] Add parentheses around the walrus assignment expression to make it valid Python --- mlx/coverity_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index 35730498..be04ecca 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -289,7 +289,7 @@ def get_defects(self, stream, filters, column_names): if filter_values: query_filters.append(self.assemble_query_filter(filter.name, filter_values, filter.matcher_type)) - if filter := filters["component"] and filter_values := self.handle_component_filter(filter): + if (filter := filters["component"]) and (filter_values := self.handle_component_filter(filter)): query_filters.append(self.assemble_query_filter("Component", filter_values, "nameMatcher")) data = { From e10b4040b4b160e644260d3ded04d87a422b1d12 Mon Sep 17 00:00:00 2001 From: JWM Date: Fri, 16 Aug 2024 13:19:19 +0200 Subject: [PATCH 14/48] Use walrus operator in other if statement --- mlx/coverity_services.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index be04ecca..44bf6627 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -282,10 +282,8 @@ def get_defects(self, stream, filters, column_names): } for option, filter in filter_options.items(): - if filters[option]: - filter_values = self.handle_attribute_filter( - filters[option], filter.name, filter.values, filter.allow_regex - ) + if (filter_option := filters[option]) and (filter_values := self.handle_attribute_filter( + filter_option, filter.name, filter.values, filter.allow_regex)): if filter_values: query_filters.append(self.assemble_query_filter(filter.name, filter_values, filter.matcher_type)) From 00d7c6c95820ea582ebf111d326d695bab421226 Mon Sep 17 00:00:00 2001 From: JWM Date: Fri, 16 Aug 2024 15:30:48 +0200 Subject: [PATCH 15/48] Use more indentation; fix flake8 --- mlx/coverity_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index 44bf6627..082858f0 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -283,7 +283,7 @@ def get_defects(self, stream, filters, column_names): for option, filter in filter_options.items(): if (filter_option := filters[option]) and (filter_values := self.handle_attribute_filter( - filter_option, filter.name, filter.values, filter.allow_regex)): + filter_option, filter.name, filter.values, filter.allow_regex)): if filter_values: query_filters.append(self.assemble_query_filter(filter.name, filter_values, filter.matcher_type)) From 7ddc617645b72bbac3a8a31681a3f5224f9dff1f Mon Sep 17 00:00:00 2001 From: JWM Date: Fri, 16 Aug 2024 15:40:11 +0200 Subject: [PATCH 16/48] Use seperate function to login and get the correct urls in tests --- tests/test_coverity.py | 88 ++++++++++++++++++++++++------------------ 1 file changed, 51 insertions(+), 37 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 467a18f1..3f10405e 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -18,6 +18,38 @@ class TestCoverity(TestCase): def setUp(self): """SetUp to be run before each test to provide clean working env""" + def initialize_coverity_service(self, login=False): + """Logs in Coverity Service and initializes the urls used for REST API. + + Returns: + CoverityDefectService: The coveritye defect service + """ + coverity_conf_service = mlx.coverity.CoverityDefectService("scan.coverity.com/") + + if login: + # Login to Coverity + coverity_conf_service.login("user", "password") + + # urls that are used in GET or POST requests + endpoint = coverity_conf_service.api_endpoint + params = { + "queryType": "bySnapshot", + "retrieveGroupByColumns": "false" + } + self.column_keys_url = f"{endpoint}/issues/columns?{urlencode(params)}" + self.checkers_url = f"{endpoint}/checkerAttributes/checker" + self.stream_url = f"{endpoint}/streams/{self.fake_stream}" + params = { + "includeColumnLabels": "true", + "offset": 0, + "queryType": "bySnapshot", + "rowCount": -1, + "sortOrder": "asc", + } + self.issues_url = f"{endpoint}/issues/search?{urlencode(params)}" + + return coverity_conf_service + @patch("mlx.coverity_services.requests") def test_session_login(self, mock_requests): """Test login function of CoverityDefectService""" @@ -45,8 +77,9 @@ def test_retrieve_checkers(self, mock_get): coverity_conf_service.retrieve_checkers() mock_get.assert_called_once() - def test_get_defects(self): - filters = { + def test_get_defects_call(self): + + self.filters = { "checker": None, "impact": None, "kind": None, @@ -56,64 +89,45 @@ def test_get_defects(self): "cwe": None, "cid": None, } - fake_json = {"test": "succes"} - fake_checkers = { + self.fake_json = {"test": "succes"} + self.fake_checkers = { "checkerAttribute": {"name": "checker", "displayName": "Checker"}, "checkerAttributedata": [{"key": "checker_key", "value": "checker_value"}], } - fake_stream = "test_stream" - coverity_conf_service = mlx.coverity.CoverityDefectService("scan.coverity.com/") - - # Login to Coverity - coverity_conf_service.login("user", "password") - # urls that are used in GET or POST requests - endpoint = coverity_conf_service.api_endpoint - params = { - "queryType": "bySnapshot", - "retrieveGroupByColumns": "false" - } - column_keys_url = f"{endpoint}/issues/columns?{urlencode(params)}" - checkers_url = f"{endpoint}/checkerAttributes/checker" - stream_url = f"{endpoint}/streams/{fake_stream}" - params = { - "includeColumnLabels": "true", - "offset": 0, - "queryType": "bySnapshot", - "rowCount": -1, - "sortOrder": "asc", - } - issues_url = f"{endpoint}/issues/search?{urlencode(params)}" + self.fake_stream = "test_stream" + # initialize what needed for the REST API + coverity_service = self.initialize_coverity_service(login=True) with requests_mock.mock() as mocker: with open("tests/columns_keys.json", "r") as content: column_keys = json.loads(content.read()) - mocker.get(column_keys_url, json=column_keys) - mocker.get(checkers_url, json=fake_checkers) - mocker.get(stream_url, json={"stream": "valid"}) - mocker.post(issues_url, json=fake_json) + mocker.get(self.column_keys_url, json=column_keys) + mocker.get(self.checkers_url, json=self.fake_checkers) + mocker.get(self.stream_url, json={"stream": "valid"}) + mocker.post(self.issues_url, json=self.fake_json) # Validate stream name - coverity_conf_service.validate_stream(fake_stream) + coverity_service.validate_stream(self.fake_stream) # Retrieve column keys - assert coverity_conf_service.retrieve_column_keys()["Issue Kind"] == "displayIssueKind" - assert coverity_conf_service.retrieve_column_keys()["CID"] == "cid" + assert coverity_service.retrieve_column_keys()["Issue Kind"] == "displayIssueKind" + assert coverity_service.retrieve_column_keys()["CID"] == "cid" # Retrieve checkers - assert coverity_conf_service.retrieve_checkers() == ["checker_key"] + assert coverity_service.retrieve_checkers() == ["checker_key"] # Get defects - assert coverity_conf_service.get_defects(fake_stream, filters, column_names=["CID"]) == fake_json + assert coverity_service.get_defects(self.fake_stream, self.filters, column_names=["CID"]) == self.fake_json # Total amount of request are 4 => column keys, checkers, stream and defects/issues assert mocker.call_count == 4 # check get requests - get_urls = [stream_url, column_keys_url, checkers_url] + get_urls = [self.stream_url, self.column_keys_url, self.checkers_url] for index in range(len(get_urls)): mock_req = mocker.request_history[index] assert mock_req.url == get_urls[index] assert mock_req.verify assert mock_req.headers["Authorization"] == requests.auth._basic_auth_str("user", "password") # check post request (last request) - assert mocker.last_request.url == issues_url + assert mocker.last_request.url == self.issues_url assert mocker.last_request.verify assert mocker.last_request.headers["Authorization"] == requests.auth._basic_auth_str("user", "password") From 38e9745634d53e72670ff75ff358ecf58b206ced Mon Sep 17 00:00:00 2001 From: JWM Date: Fri, 16 Aug 2024 15:44:08 +0200 Subject: [PATCH 17/48] Read file at begin of test --- tests/test_coverity.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 3f10405e..a7034d85 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -78,6 +78,8 @@ def test_retrieve_checkers(self, mock_get): mock_get.assert_called_once() def test_get_defects_call(self): + with open("tests/columns_keys.json", "r") as content: + column_keys = json.loads(content.read()) self.filters = { "checker": None, From 13cbd66cf774cd53c87a07be45266c068a51d214 Mon Sep 17 00:00:00 2001 From: JWM Date: Fri, 16 Aug 2024 16:51:05 +0200 Subject: [PATCH 18/48] Add test that uses the get_filtered_defects function --- tests/fake_json.json | 148 +++++++++++++++++++++++++++++++++++++++++ tests/test_coverity.py | 60 ++++++++++++++++- 2 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 tests/fake_json.json diff --git a/tests/fake_json.json b/tests/fake_json.json new file mode 100644 index 00000000..fccab282 --- /dev/null +++ b/tests/fake_json.json @@ -0,0 +1,148 @@ +{ + "offset": 0, + "totalRows": 4, + "columns": [ + "lastTriageComment", + "classification", + "checker", + "cid" + ], + "rows": [ + [ + { + "key": "lastTriageComment", + "value": "" + }, + { + "key": "classification", + "value": "Intentional" + }, + { + "key": "checker", + "value": "MISRA 1" + }, + { + "key": "cid", + "value": "189049" + }, + { + "key": "lineNumber", + "value": "25" + }, + { + "key": "displayFile", + "value": "/some/fake/file.c" + }, + { + "key": "externalReference", + "value": "REFERENCE1" + }, + { + "key": "status", + "value": "Triaged" + } + ], + [ + { + "key": "lastTriageComment", + "value": "Some fake comment" + }, + { + "key": "classification", + "value": "Intentional" + }, + { + "key": "checker", + "value": "MISRA 2" + }, + { + "key": "cid", + "value": "189050" + }, + { + "key": "lineNumber", + "value": "89" + }, + { + "key": "displayFile", + "value": "/some/fake/file.c" + }, + { + "key": "externalReference", + "value": "" + }, + { + "key": "status", + "value": "Dismissed" + } + ], + [ + { + "key": "lastTriageComment", + "value": "Some fake comment" + }, + { + "key": "classification", + "value": "Pending" + }, + { + "key": "checker", + "value": "MISRA 3" + }, + { + "key": "cid", + "value": "367264" + }, + { + "key": "lineNumber", + "value": "10" + }, + { + "key": "displayFile", + "value": "/fake/file.c" + }, + { + "key": "externalReference", + "value": "REFERENCE3" + }, + { + "key": "status", + "value": "Dismissed" + } + ], + [ + { + "key": "lastTriageComment", + "value": "Another comment" + }, + { + "key": "classification", + "value": "Pending" + }, + { + "key": "checker", + "value": "CHECK" + }, + { + "key": "cid", + "value": "367265" + }, + { + "key": "lineNumber", + "value": "34" + }, + { + "key": "displayFile", + "value": "/fake/file.c" + }, + { + "key": "externalReference", + "value": "" + }, + { + "key": "status", + "value": "Triaged" + } + ] + ] +} diff --git a/tests/test_coverity.py b/tests/test_coverity.py index a7034d85..54f17b96 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -102,8 +102,6 @@ def test_get_defects_call(self): coverity_service = self.initialize_coverity_service(login=True) with requests_mock.mock() as mocker: - with open("tests/columns_keys.json", "r") as content: - column_keys = json.loads(content.read()) mocker.get(self.column_keys_url, json=column_keys) mocker.get(self.checkers_url, json=self.fake_checkers) mocker.get(self.stream_url, json={"stream": "valid"}) @@ -133,6 +131,64 @@ def test_get_defects_call(self): assert mocker.last_request.verify assert mocker.last_request.headers["Authorization"] == requests.auth._basic_auth_str("user", "password") + def test_get_filtered_defects(self): + with open("tests/columns_keys.json", "r") as content: + column_keys = json.loads(content.read()) + + with open("tests/fake_json.json", "r") as content: + self.fake_json = json.loads(content.read()) + + self.fake_checkers = { + "checkerAttribute": { + "name": "checker", + "displayName": "Checker" + }, + "checkerAttributedata": [ + { + "key": "MISRA 1", + "value": "MISRA 1" + }, + { + "key": "MISRA 2", + "value": "MISRA 2" + }, + { + "key": "MISRA 3", + "value": "MISRA 3" + }, + { + "key": "CHECK", + "value": "CHECK" + } + ] + } + self.fake_stream = "test_stream" + + # initialize what needed for the REST API + coverity_service = self.initialize_coverity_service(login=True) + + with requests_mock.mock() as mocker: + mocker.get(self.column_keys_url, json=column_keys) + mocker.get(self.checkers_url, json=self.fake_checkers) + mocker.get(self.stream_url, json={"stream": "valid"}) + test_post = mocker.post(self.issues_url, json=self.fake_json) + + coverity_service.retrieve_checkers() + coverity_service.retrieve_column_keys() + + sphinx_coverity_connector = mlx.coverity.SphinxCoverityConnector() + sphinx_coverity_connector.coverity_service = coverity_service + sphinx_coverity_connector.stream = self.fake_stream + node_filters = {'checker': 'MISRA', 'impact': None, 'kind': None, + 'classification': 'Intentional,Bug,Pending,Unclassified', 'action': None, + 'component': None, 'cwe': None, 'cid': None} + column_names = {'Comment', 'Checker', 'Classification', 'CID'} + fake_node = {"col": column_names, + "filters": node_filters} + defects = sphinx_coverity_connector.get_filtered_defects(fake_node) + breakpoint() + assert defects == self.fake_json + def test_failed_login(self): fake_stream = "test_stream" From bd3249f3bda7028dc581be8f38535850e35d4845 Mon Sep 17 00:00:00 2001 From: JWM Date: Fri, 16 Aug 2024 16:52:15 +0200 Subject: [PATCH 19/48] Add warning if column name does not exist in the columns property --- mlx/coverity_services.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index 082858f0..a2dd7b2b 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -229,6 +229,10 @@ def assemble_query_filter(self, column_name, filter_values, matcher_type): else: matcher["key"] = filter_ matchers.append(matcher) + + if column_name not in self.columns: + self.logger.warning(f"Invalid column name {column_name!r}; Retrieve column keys first.") + return { "columnKey": self.columns[column_name], "matchMode": "oneOrMoreMatch", From 30bddc527277af5baf1804221c46de958c4056f7 Mon Sep 17 00:00:00 2001 From: JWM Date: Mon, 19 Aug 2024 08:47:22 +0200 Subject: [PATCH 20/48] Delete breakpoint --- tests/test_coverity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 54f17b96..be63d96f 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -186,7 +186,7 @@ def test_get_filtered_defects(self): fake_node = {"col": column_names, "filters": node_filters} defects = sphinx_coverity_connector.get_filtered_defects(fake_node) - breakpoint() + assert defects == self.fake_json def test_failed_login(self): From 72062405c66fd3073d902a79964f9547953e0680 Mon Sep 17 00:00:00 2001 From: JWM Date: Mon, 19 Aug 2024 08:55:03 +0200 Subject: [PATCH 21/48] Delete variable --- tests/test_coverity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index be63d96f..500a73e1 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -171,7 +171,7 @@ def test_get_filtered_defects(self): mocker.get(self.column_keys_url, json=column_keys) mocker.get(self.checkers_url, json=self.fake_checkers) mocker.get(self.stream_url, json={"stream": "valid"}) - test_post = mocker.post(self.issues_url, json=self.fake_json) + mocker.post(self.issues_url, json=self.fake_json) coverity_service.retrieve_checkers() coverity_service.retrieve_column_keys() From 17750dca53e4dc6cd467759e8c19be5aca0441e8 Mon Sep 17 00:00:00 2001 From: JWM Date: Mon, 19 Aug 2024 08:57:32 +0200 Subject: [PATCH 22/48] Use 1 indent for the dict --- tests/test_coverity.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 500a73e1..c0d8987a 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -179,10 +179,12 @@ def test_get_filtered_defects(self): sphinx_coverity_connector = mlx.coverity.SphinxCoverityConnector() sphinx_coverity_connector.coverity_service = coverity_service sphinx_coverity_connector.stream = self.fake_stream - node_filters = {'checker': 'MISRA', 'impact': None, 'kind': None, - 'classification': 'Intentional,Bug,Pending,Unclassified', 'action': None, - 'component': None, 'cwe': None, 'cid': None} - column_names = {'Comment', 'Checker', 'Classification', 'CID'} + node_filters = { + 'checker': 'MISRA', 'impact': None, 'kind': None, + 'classification': 'Intentional,Bug,Pending,Unclassified', 'action': None, 'component': None, + 'cwe': None, 'cid': None + } + column_names = {'Comment', 'Checker', 'Classification', 'CID'} fake_node = {"col": column_names, "filters": node_filters} defects = sphinx_coverity_connector.get_filtered_defects(fake_node) From fb3f407c73925bedbc8b933abfe9ce8182898624 Mon Sep 17 00:00:00 2001 From: Jasper Craeghs Date: Mon, 19 Aug 2024 13:32:31 +0200 Subject: [PATCH 23/48] Simplification of test suite and testing what matters --- tests/test_coverity.py | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index c0d8987a..c7ba2dbe 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -1,16 +1,13 @@ from unittest import TestCase - -try: - from unittest.mock import MagicMock, patch -except ImportError: - from mock import MagicMock, patch +from unittest.mock import MagicMock, patch import json import requests import requests_mock from urllib.parse import urlencode -import mlx.coverity +from mlx.coverity import SphinxCoverityConnector +from mlx.coverity_services import CoverityDefectService import mlx.coverity_services @@ -22,9 +19,9 @@ def initialize_coverity_service(self, login=False): """Logs in Coverity Service and initializes the urls used for REST API. Returns: - CoverityDefectService: The coveritye defect service + CoverityDefectService: The coverity defect service """ - coverity_conf_service = mlx.coverity.CoverityDefectService("scan.coverity.com/") + coverity_conf_service = CoverityDefectService("scan.coverity.com/") if login: # Login to Coverity @@ -56,7 +53,7 @@ def test_session_login(self, mock_requests): mock_requests.return_value = MagicMock(spec=requests) # Get the base url - coverity_conf_service = mlx.coverity_services.CoverityDefectService("scan.coverity.com/") + coverity_conf_service = CoverityDefectService("scan.coverity.com/") self.assertEqual("https://scan.coverity.com/api/v2", coverity_conf_service.api_endpoint) # Login to Coverity @@ -66,7 +63,7 @@ def test_session_login(self, mock_requests): @patch.object(mlx.coverity_services.requests.Session, "get") def test_retrieve_checkers(self, mock_get): """Test retrieving checkers (CoverityDefectService)""" - coverity_conf_service = mlx.coverity_services.CoverityDefectService("scan.coverity.com/") + coverity_conf_service = CoverityDefectService("scan.coverity.com/") # Login to Coverity coverity_conf_service.login("user", "password") @@ -91,7 +88,6 @@ def test_get_defects_call(self): "cwe": None, "cid": None, } - self.fake_json = {"test": "succes"} self.fake_checkers = { "checkerAttribute": {"name": "checker", "displayName": "Checker"}, "checkerAttributedata": [{"key": "checker_key", "value": "checker_value"}], @@ -105,17 +101,21 @@ def test_get_defects_call(self): mocker.get(self.column_keys_url, json=column_keys) mocker.get(self.checkers_url, json=self.fake_checkers) mocker.get(self.stream_url, json={"stream": "valid"}) - mocker.post(self.issues_url, json=self.fake_json) + mocker.post(self.issues_url, json={}) # Validate stream name coverity_service.validate_stream(self.fake_stream) # Retrieve column keys - assert coverity_service.retrieve_column_keys()["Issue Kind"] == "displayIssueKind" - assert coverity_service.retrieve_column_keys()["CID"] == "cid" + coverity_service.retrieve_column_keys() + assert coverity_service.columns["Issue Kind"] == "displayIssueKind" + assert coverity_service.columns["CID"] == "cid" # Retrieve checkers - assert coverity_service.retrieve_checkers() == ["checker_key"] + coverity_service.retrieve_checkers() + assert coverity_service.checkers == ["checker_key"] # Get defects - assert coverity_service.get_defects(self.fake_stream, self.filters, column_names=["CID"]) == self.fake_json + with patch.object(CoverityDefectService, "retrieve_issues") as mock_method: + coverity_service.get_defects(self.fake_stream, self.filters, column_names=["CID"]) + mock_method.assert_called_once_with('hi') # Total amount of request are 4 => column keys, checkers, stream and defects/issues assert mocker.call_count == 4 @@ -176,7 +176,7 @@ def test_get_filtered_defects(self): coverity_service.retrieve_checkers() coverity_service.retrieve_column_keys() - sphinx_coverity_connector = mlx.coverity.SphinxCoverityConnector() + sphinx_coverity_connector = SphinxCoverityConnector() sphinx_coverity_connector.coverity_service = coverity_service sphinx_coverity_connector.stream = self.fake_stream node_filters = { @@ -188,14 +188,13 @@ def test_get_filtered_defects(self): fake_node = {"col": column_names, "filters": node_filters} defects = sphinx_coverity_connector.get_filtered_defects(fake_node) - assert defects == self.fake_json def test_failed_login(self): fake_stream = "test_stream" - coverity_conf_service = mlx.coverity.CoverityDefectService("scan.coverity.com/") - stream_url = f"{coverity_conf_service.api_endpoint.rstrip('/')}/streams/{fake_stream}" + coverity_conf_service = CoverityDefectService("scan.coverity.com/") + stream_url = f"{coverity_conf_service.api_endpoint}/streams/{fake_stream}" with requests_mock.mock() as mocker: mocker.get(stream_url, headers={"Authorization": "Basic fail"}, status_code=401) From 1f2b7ad8644f9e4d7b9dfeba027edbaf918d66f4 Mon Sep 17 00:00:00 2001 From: Jasper Craeghs Date: Mon, 19 Aug 2024 13:46:08 +0200 Subject: [PATCH 24/48] Sort imports --- mlx/coverity_services.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index a2dd7b2b..803087d5 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -2,17 +2,14 @@ """Services and other utilities for Coverity scripting""" -# General -from collections import namedtuple import csv import logging import re +from collections import namedtuple from urllib.parse import urlencode -import requests.structures -from sphinx.util.logging import getLogger -# For Coverity - REST API import requests +from sphinx.util.logging import getLogger # Coverity built in Impact statuses IMPACT_LIST = ["High", "Medium", "Low"] From f53db84951d834fff5f61bd214da615225959fcd Mon Sep 17 00:00:00 2001 From: JWM <62558419+JokeWaumans@users.noreply.github.com> Date: Mon, 19 Aug 2024 14:51:47 +0200 Subject: [PATCH 25/48] Update type in docstring Co-authored-by: Jasper Craeghs <28319872+JasperCraeghs@users.noreply.github.com> --- mlx/coverity_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlx/coverity_services.py b/mlx/coverity_services.py index 803087d5..498dbad7 100644 --- a/mlx/coverity_services.py +++ b/mlx/coverity_services.py @@ -129,7 +129,7 @@ def retrieve_issues(self, filters): """Retrieve issues from the server (Coverity Connect). Args: - filters (json): The filters as json + filters (dict): The filters for the query Returns: dict: The response From 7b78da6f546848e08ed586691e9a64ea61d9a38d Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 00:05:25 +0200 Subject: [PATCH 26/48] Use parameterized to test get_defects; Add filters.py with all variables for get_defects --- tests/filters.py | 72 ++++++++++++++++++++++++++++++++++++++++++ tests/test_coverity.py | 71 ++++++++++++++++++----------------------- 2 files changed, 102 insertions(+), 41 deletions(-) create mode 100644 tests/filters.py diff --git a/tests/filters.py b/tests/filters.py new file mode 100644 index 00000000..f500913b --- /dev/null +++ b/tests/filters.py @@ -0,0 +1,72 @@ +# filters, columns_names, response data in apart python bestand + +from collections import namedtuple + +Filter = namedtuple("Filter", "filters column_names request_data") + +# Test with no filters and no column names +test_defect_filter_0 = Filter( + { + "checker": None, + "impact": None, + "kind": None, + "classification": None, + "action": None, + "component": None, + "cwe": None, + "cid": None, + }, + [], + { + "filters": [ + { + "columnKey": "streams", + "matchMode": "oneOrMoreMatch", + "matchers": [{"class": "Stream", "name": "test_stream", "type": "nameMatcher"}] + } + ], + "columns": ["cid"], + "snapshotScope": { + "show": {"scope": "last()", "includeOutdatedSnapshots": False}, + "compareTo": {"scope": "last()", "includeOutdatedSnapshots": False} + } + } +) + +test_defect_filter_1 = Filter( + { + "checker": "MISRA", + "impact": None, + "kind": None, + "classification": "Intentional,Bug,Pending,Unclassified", + "action": None, + "component": None, + "cwe": None, + "cid": None, + }, + ["CID", "Classification", "Checker", "Comment"], + {"filters": [ + {"columnKey": "streams", "matchMode": "oneOrMoreMatch", "matchers": [ + {"class": "Stream", "name": "test_stream", "type": "nameMatcher"} + ] + }, + {"columnKey": "checker", "matchMode": "oneOrMoreMatch", "matchers": [ + {"type": "keyMatcher", "key": "MISRA 2"}, + {"type": "keyMatcher", "key": "MISRA 1"}, + {"type": "keyMatcher", "key": "MISRA 3"} + ] + }, + {"columnKey": "classification", "matchMode": "oneOrMoreMatch", "matchers": [ + {"type": "keyMatcher", "key": "Bug"}, + {"type": "keyMatcher", "key": "Pending"}, + {"type": "keyMatcher", "key": "Unclassified"}, + {"type": "keyMatcher", "key": "Intentional"} + ] + } + ], + "columns": ["cid", "checker", "lastTriageComment", "classification"], + "snapshotScope": { + "show": {"scope": "last()", "includeOutdatedSnapshots": False}, + "compareTo": {"scope": "last()", "includeOutdatedSnapshots": False} + }} +) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index c7ba2dbe..ffb94a15 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -5,11 +5,22 @@ import requests import requests_mock from urllib.parse import urlencode +from parameterized import parameterized + from mlx.coverity import SphinxCoverityConnector from mlx.coverity_services import CoverityDefectService import mlx.coverity_services +from .filters import test_defect_filter_0, test_defect_filter_1 + +def ordered(obj): + if isinstance(obj, dict): + return sorted((k, ordered(v)) for k, v in obj.items()) + if isinstance(obj, list): + return sorted(ordered(x) for x in obj) + else: + return obj class TestCoverity(TestCase): def setUp(self): @@ -74,25 +85,23 @@ def test_retrieve_checkers(self, mock_get): coverity_conf_service.retrieve_checkers() mock_get.assert_called_once() - def test_get_defects_call(self): + @parameterized.expand([ + [test_defect_filter_0.filters, test_defect_filter_0.column_names, test_defect_filter_0.request_data], + [test_defect_filter_1.filters, test_defect_filter_1.column_names, test_defect_filter_1.request_data] + ]) + def test_get_defects(self, filters, column_names, request_data): with open("tests/columns_keys.json", "r") as content: column_keys = json.loads(content.read()) - - self.filters = { - "checker": None, - "impact": None, - "kind": None, - "classification": None, - "action": None, - "component": None, - "cwe": None, - "cid": None, - } self.fake_checkers = { "checkerAttribute": {"name": "checker", "displayName": "Checker"}, - "checkerAttributedata": [{"key": "checker_key", "value": "checker_value"}], + "checkerAttributedata": [ + {"key": "MISRA 1", "value": "MISRA 1"}, + {"key": "MISRA 2", "value": "MISRA 2"}, + {"key": "MISRA 3", "value": "MISRA 3"}, + {"key": "CHECKER 1", "value": "CHECKER 1"}, + {"key": "CHECKER 2", "value": "CHECKER 2"} + ], } - self.fake_stream = "test_stream" # initialize what needed for the REST API coverity_service = self.initialize_coverity_service(login=True) @@ -100,36 +109,16 @@ def test_get_defects_call(self): with requests_mock.mock() as mocker: mocker.get(self.column_keys_url, json=column_keys) mocker.get(self.checkers_url, json=self.fake_checkers) - mocker.get(self.stream_url, json={"stream": "valid"}) - mocker.post(self.issues_url, json={}) - - # Validate stream name - coverity_service.validate_stream(self.fake_stream) - # Retrieve column keys - coverity_service.retrieve_column_keys() - assert coverity_service.columns["Issue Kind"] == "displayIssueKind" - assert coverity_service.columns["CID"] == "cid" - # Retrieve checkers + # Retrieve checkers; required for get_defects() coverity_service.retrieve_checkers() - assert coverity_service.checkers == ["checker_key"] + # Retreive columns; required for get_defects() + coverity_service.retrieve_column_keys() # Get defects with patch.object(CoverityDefectService, "retrieve_issues") as mock_method: - coverity_service.get_defects(self.fake_stream, self.filters, column_names=["CID"]) - mock_method.assert_called_once_with('hi') - # Total amount of request are 4 => column keys, checkers, stream and defects/issues - assert mocker.call_count == 4 - - # check get requests - get_urls = [self.stream_url, self.column_keys_url, self.checkers_url] - for index in range(len(get_urls)): - mock_req = mocker.request_history[index] - assert mock_req.url == get_urls[index] - assert mock_req.verify - assert mock_req.headers["Authorization"] == requests.auth._basic_auth_str("user", "password") - # check post request (last request) - assert mocker.last_request.url == self.issues_url - assert mocker.last_request.verify - assert mocker.last_request.headers["Authorization"] == requests.auth._basic_auth_str("user", "password") + coverity_service.get_defects(self.fake_stream, filters, column_names) + data = mock_method.call_args[0][0] + mock_method.assert_called_once() + assert ordered(data) == ordered(request_data) def test_get_filtered_defects(self): with open("tests/columns_keys.json", "r") as content: From ac4a0e04f9ae8784375b9adaa00cee6fb2d8ff9b Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 10:19:36 +0200 Subject: [PATCH 27/48] Update test get_filtered_defects --- tests/test_coverity.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index ffb94a15..eb0a40aa 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -159,8 +159,7 @@ def test_get_filtered_defects(self): with requests_mock.mock() as mocker: mocker.get(self.column_keys_url, json=column_keys) mocker.get(self.checkers_url, json=self.fake_checkers) - mocker.get(self.stream_url, json={"stream": "valid"}) - mocker.post(self.issues_url, json=self.fake_json) + mocker.post(self.issues_url, json={}) coverity_service.retrieve_checkers() coverity_service.retrieve_column_keys() @@ -169,15 +168,18 @@ def test_get_filtered_defects(self): sphinx_coverity_connector.coverity_service = coverity_service sphinx_coverity_connector.stream = self.fake_stream node_filters = { - 'checker': 'MISRA', 'impact': None, 'kind': None, - 'classification': 'Intentional,Bug,Pending,Unclassified', 'action': None, 'component': None, - 'cwe': None, 'cid': None + "checker": "MISRA", "impact": None, "kind": None, + "classification": "Intentional,Bug,Pending,Unclassified", "action": None, "component": None, + "cwe": None, "cid": None } - column_names = {'Comment', 'Checker', 'Classification', 'CID'} + column_names = {"Comment", "Checker", "Classification", "CID"} + fake_node = CoverityDefect fake_node = {"col": column_names, "filters": node_filters} - defects = sphinx_coverity_connector.get_filtered_defects(fake_node) - assert defects == self.fake_json + + with patch.object(CoverityDefectService, "get_defects") as mock_method: + sphinx_coverity_connector.get_filtered_defects(fake_node) + mock_method.assert_called_once_with(self.fake_stream, fake_node["filters"], column_names) def test_failed_login(self): fake_stream = "test_stream" From cfc07beecf889c2bca1b09300f40a81713cf08a6 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 10:22:08 +0200 Subject: [PATCH 28/48] Delete CoverityDefect --- tests/test_coverity.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index eb0a40aa..7de939b3 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -173,7 +173,6 @@ def test_get_filtered_defects(self): "cwe": None, "cid": None } column_names = {"Comment", "Checker", "Classification", "CID"} - fake_node = CoverityDefect fake_node = {"col": column_names, "filters": node_filters} From 5614a042dffd1bcc7ad61aa4326c0a9c7bed05fb Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 10:24:12 +0200 Subject: [PATCH 29/48] Add test_retrieve_checkers --- tests/test_coverity.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 7de939b3..cfb8aded 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -85,6 +85,26 @@ def test_retrieve_checkers(self, mock_get): coverity_conf_service.retrieve_checkers() mock_get.assert_called_once() + def test_retrieve_checkers(self): + self.fake_stream = "test_stream" + self.fake_checkers = { + "checkerAttribute": {"name": "checker", "displayName": "Checker"}, + "checkerAttributedata": [ + {"key": "MISRA", "value": "MISRA"}, + {"key": "CHECKER", "value": "CHECKER"} + ], + } + # initialize what needed for the REST API + coverity_service = self.initialize_coverity_service(login=True) + + with requests_mock.mock() as mocker: + mocker.get(self.checkers_url, json=self.fake_checkers) + with patch.object(CoverityDefectService, "_request") as mock_method: + # Retrieve column keys + coverity_service.retrieve_checkers() + mock_method.assert_called_once() + assert coverity_service.checkers == ["MISRA", "CHECKERS"] + @parameterized.expand([ [test_defect_filter_0.filters, test_defect_filter_0.column_names, test_defect_filter_0.request_data], [test_defect_filter_1.filters, test_defect_filter_1.column_names, test_defect_filter_1.request_data] From 2abc4bfb624c1ea8298fe7013be6e8a03dfc675c Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 11:00:27 +0200 Subject: [PATCH 30/48] Use pathlib --- tests/test_coverity.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index cfb8aded..e801c28a 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -5,6 +5,7 @@ import requests import requests_mock from urllib.parse import urlencode +from pathlib import Path from parameterized import parameterized @@ -13,6 +14,7 @@ import mlx.coverity_services from .filters import test_defect_filter_0, test_defect_filter_1 +TEST_FOLDER = Path(__file__).parent def ordered(obj): if isinstance(obj, dict): @@ -22,6 +24,7 @@ def ordered(obj): else: return obj + class TestCoverity(TestCase): def setUp(self): """SetUp to be run before each test to provide clean working env""" @@ -110,7 +113,7 @@ def test_retrieve_checkers(self): [test_defect_filter_1.filters, test_defect_filter_1.column_names, test_defect_filter_1.request_data] ]) def test_get_defects(self, filters, column_names, request_data): - with open("tests/columns_keys.json", "r") as content: + with open(f"{TEST_FOLDER}/columns_keys.json", "r") as content: column_keys = json.loads(content.read()) self.fake_checkers = { "checkerAttribute": {"name": "checker", "displayName": "Checker"}, @@ -141,12 +144,9 @@ def test_get_defects(self, filters, column_names, request_data): assert ordered(data) == ordered(request_data) def test_get_filtered_defects(self): - with open("tests/columns_keys.json", "r") as content: + with open(f"{TEST_FOLDER}/columns_keys.json", "r") as content: column_keys = json.loads(content.read()) - with open("tests/fake_json.json", "r") as content: - self.fake_json = json.loads(content.read()) - self.fake_checkers = { "checkerAttribute": { "name": "checker", From 19843d344bf772356c67479fdd44c8987efa1f49 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 11:03:18 +0200 Subject: [PATCH 31/48] Add test_retrieve_columns --- tests/test_coverity.py | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index e801c28a..365dad0e 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -72,21 +72,23 @@ def test_session_login(self, mock_requests): # Login to Coverity coverity_conf_service.login("user", "password") - mock_requests.Session.assert_called_once() - @patch.object(mlx.coverity_services.requests.Session, "get") - def test_retrieve_checkers(self, mock_get): - """Test retrieving checkers (CoverityDefectService)""" - coverity_conf_service = CoverityDefectService("scan.coverity.com/") - - # Login to Coverity - coverity_conf_service.login("user", "password") - - with open("tests/columns_keys.json", "rb") as content: - mock_get.return_value.content = content.read() - mock_get.return_value.ok = True - coverity_conf_service.retrieve_checkers() - mock_get.assert_called_once() + def test_retrieve_columns(self): + with open(f"{TEST_FOLDER}/columns_keys.json", "r") as content: + column_keys = json.loads(content.read()) + self.fake_stream = "test_stream" + # initialize what needed for the REST API + coverity_service = self.initialize_coverity_service(login=True) + with requests_mock.mock() as mocker: + mocker.get(self.column_keys_url, json=column_keys) + coverity_service.retrieve_column_keys() + history = mocker.request_history + assert mocker.call_count == 1 + assert history[0].method == "GET" + assert history[0].url == self.column_keys_url + assert history[0].verify + assert coverity_service.columns["Issue Kind"] == "displayIssueKind" + assert coverity_service.columns["CID"] == "cid" def test_retrieve_checkers(self): self.fake_stream = "test_stream" From 15553315e4d2f420ee80c582a81a403fe75873b5 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 11:08:15 +0200 Subject: [PATCH 32/48] Update test_retrieve_checkers --- tests/test_coverity.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 365dad0e..5bd614e8 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -104,11 +104,13 @@ def test_retrieve_checkers(self): with requests_mock.mock() as mocker: mocker.get(self.checkers_url, json=self.fake_checkers) - with patch.object(CoverityDefectService, "_request") as mock_method: - # Retrieve column keys - coverity_service.retrieve_checkers() - mock_method.assert_called_once() - assert coverity_service.checkers == ["MISRA", "CHECKERS"] + coverity_service.retrieve_checkers() + history = mocker.request_history + assert mocker.call_count == 1 + assert history[0].method == "GET" + assert history[0].url == self.checkers_url + assert history[0].verify + assert coverity_service.checkers == ["MISRA", "CHECKER"] @parameterized.expand([ [test_defect_filter_0.filters, test_defect_filter_0.column_names, test_defect_filter_0.request_data], From 568e76f4adc7198c494d674c0ad88e4708d9f6a0 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 11:12:53 +0200 Subject: [PATCH 33/48] Add test_stream_validation --- tests/test_coverity.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 5bd614e8..93162d0a 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -71,7 +71,21 @@ def test_session_login(self, mock_requests): self.assertEqual("https://scan.coverity.com/api/v2", coverity_conf_service.api_endpoint) # Login to Coverity - coverity_conf_service.login("user", "password") + + @patch("mlx.coverity_services.requests") + def test_stream_validation(self, mock_requests): + mock_requests.return_value = MagicMock(spec=requests) + + self.fake_stream = "test_stream" + # Get the base url + coverity_service = CoverityDefectService("scan.coverity.com/") + # Login to Coverity + coverity_service.login("user", "password") + with patch.object(CoverityDefectService, "_request") as mock_method: + # Validate stream name + coverity_service.validate_stream(self.fake_stream) + mock_method.assert_called_once() + mock_method.assert_called_with("https://scan.coverity.com/api/v2/streams/test_stream") def test_retrieve_columns(self): with open(f"{TEST_FOLDER}/columns_keys.json", "r") as content: From 9178a53e46a48519a6f7f859a8cd1770e5be3fcc Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 11:56:33 +0200 Subject: [PATCH 34/48] Use coverity_service instead of coverity_conf_service --- tests/test_coverity.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 93162d0a..630892f9 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -35,14 +35,14 @@ def initialize_coverity_service(self, login=False): Returns: CoverityDefectService: The coverity defect service """ - coverity_conf_service = CoverityDefectService("scan.coverity.com/") + coverity_service = CoverityDefectService("scan.coverity.com/") if login: # Login to Coverity - coverity_conf_service.login("user", "password") + coverity_service.login("user", "password") # urls that are used in GET or POST requests - endpoint = coverity_conf_service.api_endpoint + endpoint = coverity_service.api_endpoint params = { "queryType": "bySnapshot", "retrieveGroupByColumns": "false" @@ -59,7 +59,7 @@ def initialize_coverity_service(self, login=False): } self.issues_url = f"{endpoint}/issues/search?{urlencode(params)}" - return coverity_conf_service + return coverity_service @patch("mlx.coverity_services.requests") def test_session_login(self, mock_requests): From 894239e2f44f22f487433b54378d23abcfea14f4 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 11:58:37 +0200 Subject: [PATCH 35/48] Update session test to validate the headers by using validate_stream --- tests/test_coverity.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 630892f9..015ebe23 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -61,16 +61,16 @@ def initialize_coverity_service(self, login=False): return coverity_service - @patch("mlx.coverity_services.requests") - def test_session_login(self, mock_requests): - """Test login function of CoverityDefectService""" - mock_requests.return_value = MagicMock(spec=requests) - - # Get the base url - coverity_conf_service = CoverityDefectService("scan.coverity.com/") - self.assertEqual("https://scan.coverity.com/api/v2", coverity_conf_service.api_endpoint) - - # Login to Coverity + def test_session_by_stream_validation(self): + self.fake_stream = "test_stream" + coverity_service = self.initialize_coverity_service(login=False) + with requests_mock.mock() as mocker: + mocker.get(self.stream_url, json={}) + # Login to Coverity + coverity_service.login("user", "password") + coverity_service.validate_stream(self.fake_stream) + stream_request = mocker.last_request + assert stream_request.headers["Authorization"] == requests.auth._basic_auth_str("user", "password") @patch("mlx.coverity_services.requests") def test_stream_validation(self, mock_requests): From 9a568619515f07b926378fe9ead0a922a75892b9 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 11:59:02 +0200 Subject: [PATCH 36/48] Use last_request instead of history[0] --- tests/test_coverity.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 015ebe23..d6266bb0 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -98,9 +98,10 @@ def test_retrieve_columns(self): coverity_service.retrieve_column_keys() history = mocker.request_history assert mocker.call_count == 1 - assert history[0].method == "GET" - assert history[0].url == self.column_keys_url - assert history[0].verify + mock_request = mocker.last_request + assert mock_request.method == "GET" + assert mock_request.url == self.column_keys_url + assert mock_request.verify assert coverity_service.columns["Issue Kind"] == "displayIssueKind" assert coverity_service.columns["CID"] == "cid" From e2c23767ef1a02b8fa11da870722621375acc254 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 11:59:35 +0200 Subject: [PATCH 37/48] Delete unused fake_json file --- tests/fake_json.json | 148 ------------------------------------------- 1 file changed, 148 deletions(-) delete mode 100644 tests/fake_json.json diff --git a/tests/fake_json.json b/tests/fake_json.json deleted file mode 100644 index fccab282..00000000 --- a/tests/fake_json.json +++ /dev/null @@ -1,148 +0,0 @@ -{ - "offset": 0, - "totalRows": 4, - "columns": [ - "lastTriageComment", - "classification", - "checker", - "cid" - ], - "rows": [ - [ - { - "key": "lastTriageComment", - "value": "" - }, - { - "key": "classification", - "value": "Intentional" - }, - { - "key": "checker", - "value": "MISRA 1" - }, - { - "key": "cid", - "value": "189049" - }, - { - "key": "lineNumber", - "value": "25" - }, - { - "key": "displayFile", - "value": "/some/fake/file.c" - }, - { - "key": "externalReference", - "value": "REFERENCE1" - }, - { - "key": "status", - "value": "Triaged" - } - ], - [ - { - "key": "lastTriageComment", - "value": "Some fake comment" - }, - { - "key": "classification", - "value": "Intentional" - }, - { - "key": "checker", - "value": "MISRA 2" - }, - { - "key": "cid", - "value": "189050" - }, - { - "key": "lineNumber", - "value": "89" - }, - { - "key": "displayFile", - "value": "/some/fake/file.c" - }, - { - "key": "externalReference", - "value": "" - }, - { - "key": "status", - "value": "Dismissed" - } - ], - [ - { - "key": "lastTriageComment", - "value": "Some fake comment" - }, - { - "key": "classification", - "value": "Pending" - }, - { - "key": "checker", - "value": "MISRA 3" - }, - { - "key": "cid", - "value": "367264" - }, - { - "key": "lineNumber", - "value": "10" - }, - { - "key": "displayFile", - "value": "/fake/file.c" - }, - { - "key": "externalReference", - "value": "REFERENCE3" - }, - { - "key": "status", - "value": "Dismissed" - } - ], - [ - { - "key": "lastTriageComment", - "value": "Another comment" - }, - { - "key": "classification", - "value": "Pending" - }, - { - "key": "checker", - "value": "CHECK" - }, - { - "key": "cid", - "value": "367265" - }, - { - "key": "lineNumber", - "value": "34" - }, - { - "key": "displayFile", - "value": "/fake/file.c" - }, - { - "key": "externalReference", - "value": "" - }, - { - "key": "status", - "value": "Triaged" - } - ] - ] -} From 349e5e6d905aec2007ea3376209bee68d4cb8156 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 12:00:04 +0200 Subject: [PATCH 38/48] Add Python package parameterized to tox.ini --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 353415a8..1a5c0504 100644 --- a/tox.ini +++ b/tox.ini @@ -39,6 +39,7 @@ deps= sphinx-testing >= 0.5.2 sphinx_selective_exclude sphinx_rtd_theme + parameterized python-decouple urlextract setuptools_scm From 9ff51df8619a1c5b48f06dcbaaff2b09d4dc6340 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 12:15:41 +0200 Subject: [PATCH 39/48] Add some more test for get_defects --- tests/filters.py | 52 ++++++++++++++++++++++++++++++++++++++++++ tests/test_coverity.py | 6 +++-- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/tests/filters.py b/tests/filters.py index f500913b..e6a2f187 100644 --- a/tests/filters.py +++ b/tests/filters.py @@ -70,3 +70,55 @@ "compareTo": {"scope": "last()", "includeOutdatedSnapshots": False} }} ) + +test_defect_filter_2 = Filter( + { + "checker": None, + "impact": None, + "kind": None, + "classification": None, + "action": None, + "component": None, + "cwe": None, + "cid": None, + }, + ["CID", "Checker", "Status", "Comment"], + {'filters': [ + {'columnKey': 'streams', 'matchMode': 'oneOrMoreMatch', 'matchers': [ + {'class': 'Stream', 'name': 'test_stream', 'type': 'nameMatcher'} + ] + }], + 'columns': ['status', 'cid', 'checker', 'lastTriageComment'], + 'snapshotScope': { + 'show': {'scope': 'last()', 'includeOutdatedSnapshots': False}, + 'compareTo': {'scope': 'last()', 'includeOutdatedSnapshots': False} + }} +) + +test_defect_filter_3 = Filter( + { + "checker": None, + "impact": None, + "kind": None, + "classification": "Unclassified", + "action": None, + "component": None, + "cwe": None, + "cid": None, + }, + ["CID", "Classification", "Action"], + {'filters': [ + {'columnKey': 'streams', 'matchMode': 'oneOrMoreMatch', 'matchers': [ + {'class': 'Stream', 'name': 'test_stream', 'type': 'nameMatcher'} + ] + }, + {'columnKey': 'classification', 'matchMode': 'oneOrMoreMatch', 'matchers': [ + {'type': 'keyMatcher', 'key': 'Unclassified'} + ] + }], + 'columns': ['cid', 'classification', 'action'], + 'snapshotScope': { + 'show': {'scope': 'last()', 'includeOutdatedSnapshots': False}, + 'compareTo': {'scope': 'last()', 'includeOutdatedSnapshots': False} + }} +) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index d6266bb0..246404ec 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -12,7 +12,7 @@ from mlx.coverity import SphinxCoverityConnector from mlx.coverity_services import CoverityDefectService import mlx.coverity_services -from .filters import test_defect_filter_0, test_defect_filter_1 +from .filters import test_defect_filter_0, test_defect_filter_1, test_defect_filter_2, test_defect_filter_3 TEST_FOLDER = Path(__file__).parent @@ -129,7 +129,9 @@ def test_retrieve_checkers(self): @parameterized.expand([ [test_defect_filter_0.filters, test_defect_filter_0.column_names, test_defect_filter_0.request_data], - [test_defect_filter_1.filters, test_defect_filter_1.column_names, test_defect_filter_1.request_data] + [test_defect_filter_1.filters, test_defect_filter_1.column_names, test_defect_filter_1.request_data], + [test_defect_filter_2.filters, test_defect_filter_2.column_names, test_defect_filter_2.request_data], + [test_defect_filter_3.filters, test_defect_filter_3.column_names, test_defect_filter_3.request_data] ]) def test_get_defects(self, filters, column_names, request_data): with open(f"{TEST_FOLDER}/columns_keys.json", "r") as content: From eee1bc196bb88d0b07ee32245717987f53b4f446 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 12:28:52 +0200 Subject: [PATCH 40/48] Format filters.py --- tests/filters.py | 118 +++++++++++++++++++++++++++-------------------- 1 file changed, 68 insertions(+), 50 deletions(-) diff --git a/tests/filters.py b/tests/filters.py index e6a2f187..b1b6dcac 100644 --- a/tests/filters.py +++ b/tests/filters.py @@ -22,15 +22,15 @@ { "columnKey": "streams", "matchMode": "oneOrMoreMatch", - "matchers": [{"class": "Stream", "name": "test_stream", "type": "nameMatcher"}] + "matchers": [{"class": "Stream", "name": "test_stream", "type": "nameMatcher"}], } ], "columns": ["cid"], "snapshotScope": { "show": {"scope": "last()", "includeOutdatedSnapshots": False}, - "compareTo": {"scope": "last()", "includeOutdatedSnapshots": False} - } - } + "compareTo": {"scope": "last()", "includeOutdatedSnapshots": False}, + }, + }, ) test_defect_filter_1 = Filter( @@ -45,30 +45,39 @@ "cid": None, }, ["CID", "Classification", "Checker", "Comment"], - {"filters": [ - {"columnKey": "streams", "matchMode": "oneOrMoreMatch", "matchers": [ - {"class": "Stream", "name": "test_stream", "type": "nameMatcher"} - ] - }, - {"columnKey": "checker", "matchMode": "oneOrMoreMatch", "matchers": [ - {"type": "keyMatcher", "key": "MISRA 2"}, - {"type": "keyMatcher", "key": "MISRA 1"}, - {"type": "keyMatcher", "key": "MISRA 3"} - ] + { + "filters": [ + { + "columnKey": "streams", + "matchMode": "oneOrMoreMatch", + "matchers": [{"class": "Stream", "name": "test_stream", "type": "nameMatcher"}], + }, + { + "columnKey": "checker", + "matchMode": "oneOrMoreMatch", + "matchers": [ + {"type": "keyMatcher", "key": "MISRA 2"}, + {"type": "keyMatcher", "key": "MISRA 1"}, + {"type": "keyMatcher", "key": "MISRA 3"}, + ], + }, + { + "columnKey": "classification", + "matchMode": "oneOrMoreMatch", + "matchers": [ + {"type": "keyMatcher", "key": "Bug"}, + {"type": "keyMatcher", "key": "Pending"}, + {"type": "keyMatcher", "key": "Unclassified"}, + {"type": "keyMatcher", "key": "Intentional"}, + ], + }, + ], + "columns": ["cid", "checker", "lastTriageComment", "classification"], + "snapshotScope": { + "show": {"scope": "last()", "includeOutdatedSnapshots": False}, + "compareTo": {"scope": "last()", "includeOutdatedSnapshots": False}, }, - {"columnKey": "classification", "matchMode": "oneOrMoreMatch", "matchers": [ - {"type": "keyMatcher", "key": "Bug"}, - {"type": "keyMatcher", "key": "Pending"}, - {"type": "keyMatcher", "key": "Unclassified"}, - {"type": "keyMatcher", "key": "Intentional"} - ] - } - ], - "columns": ["cid", "checker", "lastTriageComment", "classification"], - "snapshotScope": { - "show": {"scope": "last()", "includeOutdatedSnapshots": False}, - "compareTo": {"scope": "last()", "includeOutdatedSnapshots": False} - }} + }, ) test_defect_filter_2 = Filter( @@ -83,16 +92,20 @@ "cid": None, }, ["CID", "Checker", "Status", "Comment"], - {'filters': [ - {'columnKey': 'streams', 'matchMode': 'oneOrMoreMatch', 'matchers': [ - {'class': 'Stream', 'name': 'test_stream', 'type': 'nameMatcher'} - ] - }], - 'columns': ['status', 'cid', 'checker', 'lastTriageComment'], - 'snapshotScope': { - 'show': {'scope': 'last()', 'includeOutdatedSnapshots': False}, - 'compareTo': {'scope': 'last()', 'includeOutdatedSnapshots': False} - }} + { + "filters": [ + { + "columnKey": "streams", + "matchMode": "oneOrMoreMatch", + "matchers": [{"class": "Stream", "name": "test_stream", "type": "nameMatcher"}], + } + ], + "columns": ["status", "cid", "checker", "lastTriageComment"], + "snapshotScope": { + "show": {"scope": "last()", "includeOutdatedSnapshots": False}, + "compareTo": {"scope": "last()", "includeOutdatedSnapshots": False}, + }, + }, ) test_defect_filter_3 = Filter( @@ -107,18 +120,23 @@ "cid": None, }, ["CID", "Classification", "Action"], - {'filters': [ - {'columnKey': 'streams', 'matchMode': 'oneOrMoreMatch', 'matchers': [ - {'class': 'Stream', 'name': 'test_stream', 'type': 'nameMatcher'} - ] + { + "filters": [ + { + "columnKey": "streams", + "matchMode": "oneOrMoreMatch", + "matchers": [{"class": "Stream", "name": "test_stream", "type": "nameMatcher"}], + }, + { + "columnKey": "classification", + "matchMode": "oneOrMoreMatch", + "matchers": [{"type": "keyMatcher", "key": "Unclassified"}], + }, + ], + "columns": ["cid", "classification", "action"], + "snapshotScope": { + "show": {"scope": "last()", "includeOutdatedSnapshots": False}, + "compareTo": {"scope": "last()", "includeOutdatedSnapshots": False}, }, - {'columnKey': 'classification', 'matchMode': 'oneOrMoreMatch', 'matchers': [ - {'type': 'keyMatcher', 'key': 'Unclassified'} - ] - }], - 'columns': ['cid', 'classification', 'action'], - 'snapshotScope': { - 'show': {'scope': 'last()', 'includeOutdatedSnapshots': False}, - 'compareTo': {'scope': 'last()', 'includeOutdatedSnapshots': False} - }} + }, ) From 51bc5685da742a3d2b792dfd3dcc6f04b460d0f9 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 12:32:58 +0200 Subject: [PATCH 41/48] Delete unused import --- tests/test_coverity.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 246404ec..bbf2e18d 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -11,7 +11,6 @@ from mlx.coverity import SphinxCoverityConnector from mlx.coverity_services import CoverityDefectService -import mlx.coverity_services from .filters import test_defect_filter_0, test_defect_filter_1, test_defect_filter_2, test_defect_filter_3 TEST_FOLDER = Path(__file__).parent From fe88d925eca7370ba604c540eaf65d819bb241e5 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 12:33:47 +0200 Subject: [PATCH 42/48] Add blank line --- tests/test_coverity.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index bbf2e18d..a2513130 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -15,6 +15,7 @@ TEST_FOLDER = Path(__file__).parent + def ordered(obj): if isinstance(obj, dict): return sorted((k, ordered(v)) for k, v in obj.items()) From 1fdb8306419ee6269629b4bca6cb49e88bd94521 Mon Sep 17 00:00:00 2001 From: JWM Date: Tue, 20 Aug 2024 12:34:01 +0200 Subject: [PATCH 43/48] Delete unused variable --- tests/test_coverity.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index a2513130..e28b8d4a 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -96,7 +96,6 @@ def test_retrieve_columns(self): with requests_mock.mock() as mocker: mocker.get(self.column_keys_url, json=column_keys) coverity_service.retrieve_column_keys() - history = mocker.request_history assert mocker.call_count == 1 mock_request = mocker.last_request assert mock_request.method == "GET" From 18a54dff864652d9edd3187c88a27006df30ff64 Mon Sep 17 00:00:00 2001 From: jce Date: Wed, 21 Aug 2024 16:29:18 +0200 Subject: [PATCH 44/48] Cleanup test suite --- tests/test_coverity.py | 75 +++++++++--------------------------------- 1 file changed, 16 insertions(+), 59 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index e28b8d4a..9a667e2c 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -28,6 +28,7 @@ def ordered(obj): class TestCoverity(TestCase): def setUp(self): """SetUp to be run before each test to provide clean working env""" + self.fake_stream = "test_stream" def initialize_coverity_service(self, login=False): """Logs in Coverity Service and initializes the urls used for REST API. @@ -62,7 +63,6 @@ def initialize_coverity_service(self, login=False): return coverity_service def test_session_by_stream_validation(self): - self.fake_stream = "test_stream" coverity_service = self.initialize_coverity_service(login=False) with requests_mock.mock() as mocker: mocker.get(self.stream_url, json={}) @@ -76,7 +76,6 @@ def test_session_by_stream_validation(self): def test_stream_validation(self, mock_requests): mock_requests.return_value = MagicMock(spec=requests) - self.fake_stream = "test_stream" # Get the base url coverity_service = CoverityDefectService("scan.coverity.com/") # Login to Coverity @@ -90,7 +89,6 @@ def test_stream_validation(self, mock_requests): def test_retrieve_columns(self): with open(f"{TEST_FOLDER}/columns_keys.json", "r") as content: column_keys = json.loads(content.read()) - self.fake_stream = "test_stream" # initialize what needed for the REST API coverity_service = self.initialize_coverity_service(login=True) with requests_mock.mock() as mocker: @@ -105,7 +103,6 @@ def test_retrieve_columns(self): assert coverity_service.columns["CID"] == "cid" def test_retrieve_checkers(self): - self.fake_stream = "test_stream" self.fake_checkers = { "checkerAttribute": {"name": "checker", "displayName": "Checker"}, "checkerAttributedata": [ @@ -145,7 +142,6 @@ def test_get_defects(self, filters, column_names, request_data): {"key": "CHECKER 2", "value": "CHECKER 2"} ], } - self.fake_stream = "test_stream" # initialize what needed for the REST API coverity_service = self.initialize_coverity_service(login=True) @@ -167,64 +163,25 @@ def test_get_filtered_defects(self): with open(f"{TEST_FOLDER}/columns_keys.json", "r") as content: column_keys = json.loads(content.read()) - self.fake_checkers = { - "checkerAttribute": { - "name": "checker", - "displayName": "Checker" - }, - "checkerAttributedata": [ - { - "key": "MISRA 1", - "value": "MISRA 1" - }, - { - "key": "MISRA 2", - "value": "MISRA 2" - }, - { - "key": "MISRA 3", - "value": "MISRA 3" - }, - { - "key": "CHECK", - "value": "CHECK" - } - ] + sphinx_coverity_connector = SphinxCoverityConnector() + sphinx_coverity_connector.coverity_service = self.initialize_coverity_service(login=False) + sphinx_coverity_connector.stream = self.fake_stream + node_filters = { + "checker": "MISRA", "impact": None, "kind": None, + "classification": "Intentional,Bug,Pending,Unclassified", "action": None, "component": None, + "cwe": None, "cid": None } - self.fake_stream = "test_stream" - - # initialize what needed for the REST API - coverity_service = self.initialize_coverity_service(login=True) - - with requests_mock.mock() as mocker: - mocker.get(self.column_keys_url, json=column_keys) - mocker.get(self.checkers_url, json=self.fake_checkers) - mocker.post(self.issues_url, json={}) + column_names = {"Comment", "Checker", "Classification", "CID"} + fake_node = {"col": column_names, + "filters": node_filters} - coverity_service.retrieve_checkers() - coverity_service.retrieve_column_keys() - - sphinx_coverity_connector = SphinxCoverityConnector() - sphinx_coverity_connector.coverity_service = coverity_service - sphinx_coverity_connector.stream = self.fake_stream - node_filters = { - "checker": "MISRA", "impact": None, "kind": None, - "classification": "Intentional,Bug,Pending,Unclassified", "action": None, "component": None, - "cwe": None, "cid": None - } - column_names = {"Comment", "Checker", "Classification", "CID"} - fake_node = {"col": column_names, - "filters": node_filters} - - with patch.object(CoverityDefectService, "get_defects") as mock_method: - sphinx_coverity_connector.get_filtered_defects(fake_node) - mock_method.assert_called_once_with(self.fake_stream, fake_node["filters"], column_names) + with patch.object(CoverityDefectService, "get_defects") as mock_method: + sphinx_coverity_connector.get_filtered_defects(fake_node) + mock_method.assert_called_once_with(self.fake_stream, fake_node["filters"], column_names) def test_failed_login(self): - fake_stream = "test_stream" - coverity_conf_service = CoverityDefectService("scan.coverity.com/") - stream_url = f"{coverity_conf_service.api_endpoint}/streams/{fake_stream}" + stream_url = f"{coverity_conf_service.api_endpoint}/streams/{self.fake_stream}" with requests_mock.mock() as mocker: mocker.get(stream_url, headers={"Authorization": "Basic fail"}, status_code=401) @@ -232,5 +189,5 @@ def test_failed_login(self): coverity_conf_service.login("user", "password") # Validate stream name with self.assertRaises(requests.HTTPError) as err: - coverity_conf_service.validate_stream(fake_stream) + coverity_conf_service.validate_stream(self.fake_stream) self.assertEqual(err.exception.response.status_code, 401) From 4e8c8580dafa8be2376c658e725f4ea89da7cd5d Mon Sep 17 00:00:00 2001 From: JWM Date: Thu, 22 Aug 2024 10:08:57 +0200 Subject: [PATCH 45/48] Use more readable mocker.last_request instead of mocker.request_history --- tests/test_coverity.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index e28b8d4a..fb3246b1 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -119,11 +119,11 @@ def test_retrieve_checkers(self): with requests_mock.mock() as mocker: mocker.get(self.checkers_url, json=self.fake_checkers) coverity_service.retrieve_checkers() - history = mocker.request_history assert mocker.call_count == 1 - assert history[0].method == "GET" - assert history[0].url == self.checkers_url - assert history[0].verify + mock_request = mocker.last_request + assert mock_request.method == "GET" + assert mock_request.url == self.checkers_url + assert mock_request.verify assert coverity_service.checkers == ["MISRA", "CHECKER"] @parameterized.expand([ From b51936e39f3bea35eb6a4745d2cfe537594ef0bd Mon Sep 17 00:00:00 2001 From: JWM <62558419+JokeWaumans@users.noreply.github.com> Date: Thu, 22 Aug 2024 10:09:59 +0200 Subject: [PATCH 46/48] Use Filter named tuple directly instead of the arguments Co-authored-by: Jasper Craeghs <28319872+JasperCraeghs@users.noreply.github.com> --- tests/test_coverity.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 9a667e2c..22a13957 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -124,10 +124,10 @@ def test_retrieve_checkers(self): assert coverity_service.checkers == ["MISRA", "CHECKER"] @parameterized.expand([ - [test_defect_filter_0.filters, test_defect_filter_0.column_names, test_defect_filter_0.request_data], - [test_defect_filter_1.filters, test_defect_filter_1.column_names, test_defect_filter_1.request_data], - [test_defect_filter_2.filters, test_defect_filter_2.column_names, test_defect_filter_2.request_data], - [test_defect_filter_3.filters, test_defect_filter_3.column_names, test_defect_filter_3.request_data] + test_defect_filter_0, + test_defect_filter_1, + test_defect_filter_2, + test_defect_filter_3, ]) def test_get_defects(self, filters, column_names, request_data): with open(f"{TEST_FOLDER}/columns_keys.json", "r") as content: From bfd8fbf3a26fa53e55408883acdd5f2b201569d5 Mon Sep 17 00:00:00 2001 From: JWM Date: Thu, 22 Aug 2024 10:24:48 +0200 Subject: [PATCH 47/48] Make key and value of fake checkers different; Only the keys will be taken to match --- tests/filters.py | 2 +- tests/test_coverity.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/filters.py b/tests/filters.py index b1b6dcac..95711ea3 100644 --- a/tests/filters.py +++ b/tests/filters.py @@ -56,7 +56,7 @@ "columnKey": "checker", "matchMode": "oneOrMoreMatch", "matchers": [ - {"type": "keyMatcher", "key": "MISRA 2"}, + {"type": "keyMatcher", "key": "MISRA 2 KEY"}, {"type": "keyMatcher", "key": "MISRA 1"}, {"type": "keyMatcher", "key": "MISRA 3"}, ], diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 8bc04b03..c56f8bca 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -135,11 +135,11 @@ def test_get_defects(self, filters, column_names, request_data): self.fake_checkers = { "checkerAttribute": {"name": "checker", "displayName": "Checker"}, "checkerAttributedata": [ - {"key": "MISRA 1", "value": "MISRA 1"}, - {"key": "MISRA 2", "value": "MISRA 2"}, - {"key": "MISRA 3", "value": "MISRA 3"}, - {"key": "CHECKER 1", "value": "CHECKER 1"}, - {"key": "CHECKER 2", "value": "CHECKER 2"} + {"key": "MISRA 1", "value": "M 1"}, + {"key": "MISRA 2 KEY", "value": "MISRA 2 VALUE"}, + {"key": "MISRA 3", "value": "M 3"}, + {"key": "C 1", "value": "CHECKER 1"}, + {"key": "C 2", "value": "CHECKER 2"} ], } # initialize what needed for the REST API From 61ae1048a0c16b3c26f8999680c5a6b13748579f Mon Sep 17 00:00:00 2001 From: JWM Date: Thu, 22 Aug 2024 10:29:34 +0200 Subject: [PATCH 48/48] Delete unused column_keys --- tests/test_coverity.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index c56f8bca..7b0b9397 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -160,9 +160,6 @@ def test_get_defects(self, filters, column_names, request_data): assert ordered(data) == ordered(request_data) def test_get_filtered_defects(self): - with open(f"{TEST_FOLDER}/columns_keys.json", "r") as content: - column_keys = json.loads(content.read()) - sphinx_coverity_connector = SphinxCoverityConnector() sphinx_coverity_connector.coverity_service = self.initialize_coverity_service(login=False) sphinx_coverity_connector.stream = self.fake_stream