From 5a8d601fa621d2cc60c87ca33e09ea79172b40dd Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Wed, 22 May 2024 10:24:03 -0500 Subject: [PATCH 1/2] Run black locally and validate that tox fix works --- plugins/module_utils/service_catalog.py | 56 +++++++++++--- plugins/modules/service_catalog_info.py | 6 +- .../plugins/module_utils/test_attachment.py | 74 ++++++++++--------- .../modules/test_service_catalog_info.py | 37 ++++++---- 4 files changed, 111 insertions(+), 62 deletions(-) diff --git a/plugins/module_utils/service_catalog.py b/plugins/module_utils/service_catalog.py index 199918cf..14273295 100644 --- a/plugins/module_utils/service_catalog.py +++ b/plugins/module_utils/service_catalog.py @@ -51,8 +51,15 @@ def sys_id(self): class Catalog(ServiceCatalogObject): - DISPLAY_FIELDS = ["sys_id", "description", "title", - "has_categories", "has_items", "categories", "sn_items"] + DISPLAY_FIELDS = [ + "sys_id", + "description", + "title", + "has_categories", + "has_items", + "categories", + "sn_items", + ] def __init__(self, data=None): if not data: @@ -85,8 +92,13 @@ def to_ansible(self): class Category(ServiceCatalogObject): - DISPLAY_FIELDS = ["sys_id", "description", "title", - "full_description", "subcategories"] + DISPLAY_FIELDS = [ + "sys_id", + "description", + "title", + "full_description", + "subcategories", + ] def __init__(self, data=None): if not data: @@ -96,10 +108,22 @@ def __init__(self, data=None): class Item(ServiceCatalogObject): - DISPLAY_FIELDS = ["sys_id", "short_description", "description", - "availability", "mandatory_attachment", "request_method", - "type", "sys_class_name", "catalogs", "name", "category", "order", - "categories", "variables"] + DISPLAY_FIELDS = [ + "sys_id", + "short_description", + "description", + "availability", + "mandatory_attachment", + "request_method", + "type", + "sys_class_name", + "catalogs", + "name", + "category", + "order", + "categories", + "variables", + ] def __init__(self, data=None): if not data: @@ -127,7 +151,9 @@ def get_catalog(self, id): """Returns the catalog identified by id""" if not id: raise ValueError("catalog sys_id is missing") - record = self.generic_client.get_record_by_sys_id("/".join([SN_BASE_PATH, "catalogs"]), id) + record = self.generic_client.get_record_by_sys_id( + "/".join([SN_BASE_PATH, "catalogs"]), id + ) if record: return Catalog(record) return None @@ -137,7 +163,8 @@ def get_categories(self, catalog_id): if not id: raise ValueError("catalog sys_id is missing") records = self.generic_client.list_records( - "/".join([SN_BASE_PATH, "catalogs", catalog_id, "categories"])) + "/".join([SN_BASE_PATH, "catalogs", catalog_id, "categories"]) + ) if records: return [Category(record) for record in records] return [] @@ -151,7 +178,8 @@ def get_items(self, catalog_id, query=None, batch_size=1000): _query.update(query) self.generic_client.batch_size = batch_size records = self.generic_client.list_records( - "/".join([SN_BASE_PATH, "items"]), _query) + "/".join([SN_BASE_PATH, "items"]), _query + ) if records: return [Item(record) for record in records] return [] @@ -159,4 +187,8 @@ def get_items(self, catalog_id, query=None, batch_size=1000): def get_item(self, id): if not id: raise ValueError("item sys_id is missing") - return Item(self.generic_client.get_record_by_sys_id("/".join([SN_BASE_PATH, "items"]), id)) + return Item( + self.generic_client.get_record_by_sys_id( + "/".join([SN_BASE_PATH, "items"]), id + ) + ) diff --git a/plugins/modules/service_catalog_info.py b/plugins/modules/service_catalog_info.py index be853680..b40a5005 100644 --- a/plugins/modules/service_catalog_info.py +++ b/plugins/modules/service_catalog_info.py @@ -172,7 +172,7 @@ def get_catalog_info(sc_client, catalog, with_categories, items_config): def run(module, sc_client): items_config = dict( info=ItemContent.from_str(module.params["items_info"]), - query=module.params["items_query"] + query=module.params["items_query"], ) fetch_categories = module.params["categories"] @@ -182,7 +182,7 @@ def run(module, sc_client): sc_client, sc_client.get_catalog(module.params["sys_id"]), fetch_categories, - items_config + items_config, ) return [catalog.to_ansible()] @@ -210,7 +210,7 @@ def main(): choices=["brief", "full", "none"], default="none", ), - items_query=dict(type="str") + items_query=dict(type="str"), ) module = AnsibleModule( diff --git a/tests/unit/plugins/module_utils/test_attachment.py b/tests/unit/plugins/module_utils/test_attachment.py index 2cd759f7..b4a0092b 100644 --- a/tests/unit/plugins/module_utils/test_attachment.py +++ b/tests/unit/plugins/module_utils/test_attachment.py @@ -179,44 +179,50 @@ def test_duplicate(self, tmp_path): class TestAttachmentAreChanged: def test_unchanged(self): - assert attachment.are_changed( - [ - {"hash": "hash", "file_name": "attachment_name.txt"}, - {"hash": "hash", "file_name": "another_file_name.txt"}, - ], - { - "attachment_name.txt": { - "path": "some/path/file_name.txt", - "type": "text/markdown", - "hash": "hash", - }, - "another_file_name.txt": { - "path": "some/path/another_file_name.txt", - "type": "text/plain", - "hash": "hash", + assert ( + attachment.are_changed( + [ + {"hash": "hash", "file_name": "attachment_name.txt"}, + {"hash": "hash", "file_name": "another_file_name.txt"}, + ], + { + "attachment_name.txt": { + "path": "some/path/file_name.txt", + "type": "text/markdown", + "hash": "hash", + }, + "another_file_name.txt": { + "path": "some/path/another_file_name.txt", + "type": "text/plain", + "hash": "hash", + }, }, - }, - ) == [False, False] + ) + == [False, False] + ) def test_changed(self): - assert attachment.are_changed( - [ - {"hash": "oldhash", "file_name": "attachment_name.txt"}, - {"hash": "oldhash", "file_name": "another_file_name.txt"}, - ], - { - "attachment_name.txt": { - "path": "some/path/file_name.txt", - "type": "text/markdown", - "hash": "hash", - }, - "another_file_name.txt": { - "path": "some/path/another_file_name.txt", - "type": "text/plain", - "hash": "hash", + assert ( + attachment.are_changed( + [ + {"hash": "oldhash", "file_name": "attachment_name.txt"}, + {"hash": "oldhash", "file_name": "another_file_name.txt"}, + ], + { + "attachment_name.txt": { + "path": "some/path/file_name.txt", + "type": "text/markdown", + "hash": "hash", + }, + "another_file_name.txt": { + "path": "some/path/another_file_name.txt", + "type": "text/plain", + "hash": "hash", + }, }, - }, - ) == [True, True] + ) + == [True, True] + ) class TestAttachmentListRecords: diff --git a/tests/unit/plugins/modules/test_service_catalog_info.py b/tests/unit/plugins/modules/test_service_catalog_info.py index 84a06508..d33788ae 100644 --- a/tests/unit/plugins/modules/test_service_catalog_info.py +++ b/tests/unit/plugins/modules/test_service_catalog_info.py @@ -10,9 +10,7 @@ import sys import pytest -from ansible_collections.servicenow.itsm.plugins.module_utils import ( - service_catalog -) +from ansible_collections.servicenow.itsm.plugins.module_utils import service_catalog from ansible_collections.servicenow.itsm.plugins.modules import ( service_catalog_info, ) @@ -33,11 +31,15 @@ def list_records(self, api, query=None): self.call_params.append(query) if "categories" in api: if not isinstance(self.retured_data, tuple): - raise ValueError("expected tuple in retured_data when looking for categories") + raise ValueError( + "expected tuple in retured_data when looking for categories" + ) return self.retured_data[1] if "items" in api: if not isinstance(self.retured_data, tuple): - raise ValueError("expected tuple in retured_data when looking for categories") + raise ValueError( + "expected tuple in retured_data when looking for categories" + ) return self.retured_data[2] if isinstance(self.retured_data, tuple): return self.retured_data[0] @@ -59,7 +61,7 @@ def test_get_without_categories(self, create_module): ), categories=False, items_info="none", - items_query=None + items_query=None, ) ) @@ -85,7 +87,7 @@ def test_get_by_sys_id(self, create_module): sys_id="catalog_sys_id", categories=False, items_info="none", - items_query=None + items_query=None, ) ) @@ -110,7 +112,7 @@ def test_get_with_categories(self, create_module): ), categories=True, items_info="none", - items_query=None + items_query=None, ) ) @@ -123,11 +125,16 @@ def test_get_with_categories(self, create_module): ) category = dict(sys_id="category_sys_id") - records = service_catalog_info.run(module, self.get_sc_client(([catalog], [category]))) + records = service_catalog_info.run( + module, self.get_sc_client(([catalog], [category])) + ) assert len(records) == 1 assert len(records[0]["categories"]) == 1 - assert records[0]["categories"][0] == service_catalog.Category(category).to_ansible() + assert ( + records[0]["categories"][0] + == service_catalog.Category(category).to_ansible() + ) assert records[0]["sys_id"] == "1" def test_get_with_categories_and_items(self, create_module): @@ -138,7 +145,7 @@ def test_get_with_categories_and_items(self, create_module): ), categories=True, items_info="brief", - items_query=None + items_query=None, ) ) @@ -153,11 +160,15 @@ def test_get_with_categories_and_items(self, create_module): item = dict(sys_id="item_sys_id") records = service_catalog_info.run( - module, self.get_sc_client(([catalog], [category], [item]))) + module, self.get_sc_client(([catalog], [category], [item])) + ) assert len(records) == 1 assert len(records[0]["categories"]) == 1 assert len(records[0]["sn_items"]) == 1 - assert records[0]["categories"][0] == service_catalog.Category(category).to_ansible() + assert ( + records[0]["categories"][0] + == service_catalog.Category(category).to_ansible() + ) assert records[0]["sn_items"][0] == service_catalog.Category(item).to_ansible() assert records[0]["sys_id"] == "1" From 6ba8fd2bf302b0d5c952698a405203b85913b72b Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Wed, 22 May 2024 10:32:14 -0500 Subject: [PATCH 2/2] Correct arg passing --- .github/workflows/linters.yml | 4 +++- .github/workflows/tox.yml | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml index b701de70..9c17afe5 100644 --- a/.github/workflows/linters.yml +++ b/.github/workflows/linters.yml @@ -6,9 +6,11 @@ on: [workflow_call] # allow this workflow to be called from other workflows jobs: linters: uses: ./.github/workflows/tox.yml + env: + source_directory: "./source" with: envname: "" labelname: "lint" checkout_path: ${{ env.source_directory }} - checkout_ref: ${{ github.event.pull_request.head.sha }} + checkout_ref: ${{ github.event.pull_request.head.sha }} checkout_fetch_depth: "0" diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index 05edcc6f..7e8c8e23 100644 --- a/.github/workflows/tox.yml +++ b/.github/workflows/tox.yml @@ -16,7 +16,7 @@ on: description: Path to pass to checkout action required: false type: string - default: ${{ env.source_directory }} + default: "" checkout_ref: description: Repository reference to pass to checkout action required: false