From b1902cbe37206d2dcab06df4ccd8fd1c44977658 Mon Sep 17 00:00:00 2001 From: tcm5343 <48961675+tcm5343@users.noreply.github.com> Date: Fri, 7 Feb 2025 00:58:35 -0600 Subject: [PATCH 1/9] rename logging config --- src/cycl/cli.py | 4 ++-- src/cycl/utils/log_config.py | 2 +- tests/cycl/cli_test.py | 8 ++++---- tests/cycl/utils/log_config_test.py | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/cycl/cli.py b/src/cycl/cli.py index c714bca..9ac51a6 100644 --- a/src/cycl/cli.py +++ b/src/cycl/cli.py @@ -5,7 +5,7 @@ import networkx as nx from cycl import build_dependency_graph -from cycl.utils.log_config import configure_logging +from cycl.utils.log_config import configure_log def app() -> None: @@ -24,7 +24,7 @@ def app() -> None: subparsers.add_parser('check', parents=[parent_parser], help='Check for cycles in AWS stack imports/exports') args = parser.parse_args() - configure_logging(getattr(logging, args.log_level)) + configure_log(getattr(logging, args.log_level)) if args.action == 'check': cycle_found = False diff --git a/src/cycl/utils/log_config.py b/src/cycl/utils/log_config.py index 08c2022..e1f5b12 100644 --- a/src/cycl/utils/log_config.py +++ b/src/cycl/utils/log_config.py @@ -1,6 +1,6 @@ import logging -def configure_logging(level: int) -> None: +def configure_log(level: int) -> None: log_format = '%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d: %(message)s' logging.basicConfig(level=level, format=log_format, datefmt='%Y-%m-%d %H:%M:%S') diff --git a/tests/cycl/cli_test.py b/tests/cycl/cli_test.py index 8bc7390..ed7ca08 100644 --- a/tests/cycl/cli_test.py +++ b/tests/cycl/cli_test.py @@ -17,8 +17,8 @@ def mock_build_build_dependency_graph(): @pytest.fixture(autouse=True) -def mock_configure_logging(): - with patch.object(cli_module, 'configure_logging') as mock: +def mock_configure_log(): + with patch.object(cli_module, 'configure_log') as mock: yield mock @@ -78,11 +78,11 @@ def test_app_check_cyclic_exit_zero(capsys, mock_build_build_dependency_graph): ('WARNING', logging.WARNING), ], ) -def test_app_check_acyclic_log_level(mock_configure_logging, arg_value, log_level): +def test_app_check_acyclic_log_level(mock_configure_log, arg_value, log_level): sys.argv = ['cycl', 'check', '--log-level', arg_value] with pytest.raises(SystemExit) as err: app() assert err.value.code == 0 - mock_configure_logging.assert_called_with(log_level) + mock_configure_log.assert_called_with(log_level) diff --git a/tests/cycl/utils/log_config_test.py b/tests/cycl/utils/log_config_test.py index e26a78b..3607fa8 100644 --- a/tests/cycl/utils/log_config_test.py +++ b/tests/cycl/utils/log_config_test.py @@ -1,10 +1,10 @@ import logging -from cycl.utils.log_config import configure_logging +from cycl.utils.log_config import configure_log def test_logging_format(caplog): - configure_logging(logging.INFO) + configure_log(logging.INFO) with caplog.at_level(logging.INFO): logging.info('Test log message') From 4a8a84d16b43a87221bac097f38a89d40f185f75 Mon Sep 17 00:00:00 2001 From: tcm5343 <48961675+tcm5343@users.noreply.github.com> Date: Fri, 7 Feb 2025 20:01:16 -0600 Subject: [PATCH 2/9] structure change --- .github/workflows/ci.yml | 4 +- .gitignore | 3 + Makefile | 35 +++++- pyproject.toml | 31 +++++ requirements.txt | 3 +- tests/cycl/__init__.py | 0 tests/cycl/cli_test.py | 88 ------------- tests/cycl/cycl_test.py | 187 ---------------------------- tests/cycl/utils/__init__.py | 0 tests/cycl/utils/cfn_test.py | 165 ------------------------ tests/cycl/utils/log_config_test.py | 17 --- 11 files changed, 70 insertions(+), 463 deletions(-) delete mode 100644 tests/cycl/__init__.py delete mode 100644 tests/cycl/cli_test.py delete mode 100644 tests/cycl/cycl_test.py delete mode 100644 tests/cycl/utils/__init__.py delete mode 100644 tests/cycl/utils/cfn_test.py delete mode 100644 tests/cycl/utils/log_config_test.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1ccb311..2fc85e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,8 +16,8 @@ jobs: matrix: os: - 'ubuntu-latest' - # - 'windows-latest' - # - 'mac-latest' + - 'windows-latest' + - 'mac-latest' runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 diff --git a/.gitignore b/.gitignore index 9f4fa5d..afbe948 100644 --- a/.gitignore +++ b/.gitignore @@ -178,3 +178,6 @@ volume # aws aws + +# make marker files +requirements*.hash diff --git a/Makefile b/Makefile index eda920a..deddf34 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,40 @@ +SHELL := /bin/bash +.SHELLFLAGS = -ec +.DEFAULT_GOAL = help +.PHONY = help clean format test install-deps + +REQ_FILES := requirements.txt requirements-dev.txt +REQ_HASH := requirements.hash + help: @egrep -h '\s##\s' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m %-30s\033[0m %s\n", $$1, $$2}' -format: ## format the project +# Generate a hash file based on the last modified times of REQ_FILES +# HANDLE IF ERROR TO CLEAN REQ_HASH +$(REQ_HASH): $(REQ_FILES) + @echo "Checking if dependencies need to be installed..." + stat -c %Y $(REQ_FILES) | md5sum | awk '{print $$1}' > $(REQ_HASH) + @echo "Installing dependencies..." + python3 -m venv ./.venv + pip install -e .[dev] + +install-deps: $(REQ_HASH) ## Install dependencies if requirements files changed + +format: ## Format the project ruff format ruff check --fix -test: ## run unit tests and generate coverage report +validate: ## Validate the project is linted and formatted + ruff format --check + ruff check + +test: ## Run unit tests and generate coverage report pytest --cov=./src/ ./tests/ + +clean: ## Clean generated project files + rm -f $(REQ_HASH) + rm -f .coverage + rm -rf ./.ruff_cache + rm -rf ./.pytest_cache + # rm -rf ./.venv + find . -type d -name "__pycache__" -exec rm -r {} + diff --git a/pyproject.toml b/pyproject.toml index 92a00cd..240c9b8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,6 +15,19 @@ classifiers = [ 'Programming Language :: Python', 'Topic :: Software Development', ] +dependencies = [ + 'boto3~=1.0', + 'networkx~=3.0', +] + +[project.optional-dependencies] +dev = [ + 'pytest-cov==6.0.0', + 'pytest-subtests==0.14.1', + 'pytest-sugar==1.0.0', + 'pytest==8.3.4', + 'ruff==0.9.4', +] [project.urls] Repository = 'http://github.com/tcm5343/cycl' @@ -69,3 +82,21 @@ omit = [ [tool.pytest.ini_options] log_cli_level = "INFO" +addopts = ['--import-mode=importlib'] + +[tool.tox] +requires = ["tox>=4.19"] +setenv = 'VIRTUALENV_DISCOVERY=pyenv' +env_list = [ + "3.8", + "3.9", + "3.10", + "3.11", + "3.12", + "3.13", +] + +[tool.tox.env_run_base] +allowlist_externals = ['pytest'] +description = "Run test under {base_python}" +commands = [["pytest", '--cov=./src/', './tests/']] diff --git a/requirements.txt b/requirements.txt index f15e98e..8b13789 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1 @@ -boto3==1.36.14 -networkx==3.4.2 + diff --git a/tests/cycl/__init__.py b/tests/cycl/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/cycl/cli_test.py b/tests/cycl/cli_test.py deleted file mode 100644 index ed7ca08..0000000 --- a/tests/cycl/cli_test.py +++ /dev/null @@ -1,88 +0,0 @@ -import logging -import sys -from unittest.mock import patch - -import networkx as nx -import pytest - -import cycl.cli as cli_module -from cycl.cli import app - - -@pytest.fixture(autouse=True) -def mock_build_build_dependency_graph(): - with patch.object(cli_module, 'build_dependency_graph') as mock: - mock.return_value = nx.MultiDiGraph() - yield mock - - -@pytest.fixture(autouse=True) -def mock_configure_log(): - with patch.object(cli_module, 'configure_log') as mock: - yield mock - - -def test_app_check_acyclic(): - sys.argv = ['cycl', 'check'] - with pytest.raises(SystemExit) as err: - app() - - assert err.value.code == 0 - - -def test_app_check_cyclic(capsys, mock_build_build_dependency_graph): - graph = nx.MultiDiGraph() - graph.add_edges_from( - [ - (1, 2), - (2, 1), - ] - ) - mock_build_build_dependency_graph.return_value = graph - sys.argv = ['cycl', 'check'] - - with pytest.raises(SystemExit) as err: - app() - - assert err.value.code == 1 - console_output = capsys.readouterr().out - assert 'Cycle found between nodes: [1, 2]' in console_output - - -def test_app_check_cyclic_exit_zero(capsys, mock_build_build_dependency_graph): - graph = nx.MultiDiGraph() - graph.add_edges_from( - [ - (1, 2), - (2, 1), - ] - ) - mock_build_build_dependency_graph.return_value = graph - sys.argv = ['cycl', 'check', '--exit-zero'] - - with pytest.raises(SystemExit) as err: - app() - - assert err.value.code == 0 - console_output = capsys.readouterr().out - assert 'Cycle found between nodes: [1, 2]' in console_output - - -@pytest.mark.parametrize( - ('arg_value', 'log_level'), - [ - ('CRITICAL', logging.CRITICAL), - ('DEBUG', logging.DEBUG), - ('ERROR', logging.ERROR), - ('INFO', logging.INFO), - ('WARNING', logging.WARNING), - ], -) -def test_app_check_acyclic_log_level(mock_configure_log, arg_value, log_level): - sys.argv = ['cycl', 'check', '--log-level', arg_value] - - with pytest.raises(SystemExit) as err: - app() - - assert err.value.code == 0 - mock_configure_log.assert_called_with(log_level) diff --git a/tests/cycl/cycl_test.py b/tests/cycl/cycl_test.py deleted file mode 100644 index 7a489a6..0000000 --- a/tests/cycl/cycl_test.py +++ /dev/null @@ -1,187 +0,0 @@ -from unittest.mock import patch - -import networkx as nx -import pytest - -import cycl.cycl as cycl_module -from cycl.cycl import build_dependency_graph - - -@pytest.fixture(autouse=True) -def mock_parse_name_from_id(): - with patch.object(cycl_module, 'parse_name_from_id') as mock: - mock.side_effect = lambda x: x - yield mock - - -@pytest.fixture(autouse=True) -def mock_get_all_exports(): - with patch.object(cycl_module, 'get_all_exports') as mock: - mock.return_value = [] - yield mock - - -@pytest.fixture(autouse=True) -def mock_get_all_imports(): - with patch.object(cycl_module, 'get_all_imports') as mock: - mock.return_value = [] - yield mock - - -def test_build_dependency_graph_returns_empty_graph(): - actual_graph = build_dependency_graph() - - assert nx.number_of_nodes(actual_graph) == 0 - assert nx.is_directed_acyclic_graph(actual_graph) - assert next(nx.simple_cycles(actual_graph), []) == [] - - -def test_build_dependency_graph_returns_graph(mock_get_all_exports, mock_get_all_imports, subtests): - """ - Visual representation of expected output graph: - some-exporting-stack-id-1 - │ - ├──► some-importing-stack-name-1 - │ - ├──► some-importing-stack-name-2 - """ - mock_get_all_exports.return_value = [ - { - 'ExportingStackId': 'some-exporting-stack-id-1', - 'Name': 'some-name-1', - 'Value': 'some-value-1', - } - ] - mock_get_all_imports.return_value = [ - 'some-importing-stack-name-1', - 'some-importing-stack-name-2', - ] - expected_edges = [ - ('some-exporting-stack-id-1', 'some-importing-stack-name-1'), - ('some-exporting-stack-id-1', 'some-importing-stack-name-2'), - ] - expected_nodes = [ - 'some-exporting-stack-id-1', - 'some-importing-stack-name-1', - 'some-importing-stack-name-2', - ] - - actual_graph = build_dependency_graph() - - for expected_node in expected_nodes: - with subtests.test(msg='assert graph has node', expected_node=expected_node): - assert actual_graph.has_node(expected_node) - assert nx.number_of_nodes(actual_graph) == len(expected_nodes) - - for expected_edge in expected_edges: - with subtests.test(msg='assert graph has edge', expected_edge=expected_edge): - assert actual_graph.has_edge(*expected_edge) - assert nx.number_of_edges(actual_graph) == len(expected_edges) - - assert nx.is_directed_acyclic_graph(actual_graph) - assert next(nx.simple_cycles(actual_graph), []) == [] - - -def test_build_dependency_graph_returns_graph_with_multiple_exports(mock_get_all_exports, mock_get_all_imports, subtests): - """ - Visual representation of expected output graph: - some-exporting-stack-id-1 - │ - ├──► some-importing-stack-name-1 - │ - ├──► some-importing-stack-name-2 - - some-exporting-stack-id-2 - │ - ├──► some-importing-stack-name-1 - - some-exporting-stack-id-3 (No outgoing edges) - some-importing-stack-name-1 (No outgoing edges) - some-importing-stack-name-2 (No outgoing edges) - """ - mock_get_all_exports.return_value = [ - { - 'ExportingStackId': 'some-exporting-stack-id-1', - 'Name': 'some-name-1', - 'Value': 'some-value-1', - }, - { - 'ExportingStackId': 'some-exporting-stack-id-2', - 'Name': 'some-name-2', - 'Value': 'some-value-2', - }, - ] - - def mock_get_all_imports_side_effect_func(export_name): - if export_name == 'some-name-1': - return [ - 'some-importing-stack-name-1', - 'some-importing-stack-name-2', - ] - if export_name == 'some-name-2': - return [ - 'some-importing-stack-name-1', - ] - return [] # handles export with no import - - mock_get_all_imports.side_effect = mock_get_all_imports_side_effect_func - expected_edges = [ - ('some-exporting-stack-id-1', 'some-importing-stack-name-1'), - ('some-exporting-stack-id-1', 'some-importing-stack-name-2'), - ('some-exporting-stack-id-2', 'some-importing-stack-name-1'), - ] - expected_nodes = [ - 'some-exporting-stack-id-1', - 'some-exporting-stack-id-2', - 'some-importing-stack-name-1', - 'some-importing-stack-name-2', - ] - - actual_graph = build_dependency_graph() - - for expected_node in expected_nodes: - with subtests.test(msg='assert graph has node', expected_node=expected_node): - assert actual_graph.has_node(expected_node) - assert nx.number_of_nodes(actual_graph) == len(expected_nodes) - - for expected_edge in expected_edges: - with subtests.test(msg='assert graph has edge', expected_edge=expected_edge): - assert actual_graph.has_edge(*expected_edge) - assert nx.number_of_edges(actual_graph) == len(expected_edges) - - assert nx.is_directed_acyclic_graph(actual_graph) - assert next(nx.simple_cycles(actual_graph), []) == [] - - -def test_build_dependency_graph_returns_graph_when_export_has_no_imports( - mock_get_all_exports, mock_get_all_imports, subtests -): - """ - Visual representation of expected output graph: - some-exporting-stack-id-1 (No outgoing edges) - """ - mock_get_all_exports.return_value = [ - { - 'ExportingStackId': 'some-exporting-stack-id-1', - 'Name': 'some-name-1', - 'Value': 'some-value-1', - }, - ] - mock_get_all_imports.return_value = [] - expected_edges = [] - expected_nodes = ['some-exporting-stack-id-1'] - - actual_graph = build_dependency_graph() - - for expected_node in expected_nodes: - with subtests.test(msg='assert graph has node', expected_node=expected_node): - assert actual_graph.has_node(expected_node) - assert nx.number_of_nodes(actual_graph) == len(expected_nodes) - - for expected_edge in expected_edges: - with subtests.test(msg='assert graph has edge', expected_edge=expected_edge): - assert actual_graph.has_edge(*expected_edge) - assert nx.number_of_edges(actual_graph) == len(expected_edges) - - assert nx.is_directed_acyclic_graph(actual_graph) - assert next(nx.simple_cycles(actual_graph), []) == [] diff --git a/tests/cycl/utils/__init__.py b/tests/cycl/utils/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/cycl/utils/cfn_test.py b/tests/cycl/utils/cfn_test.py deleted file mode 100644 index 28244a9..0000000 --- a/tests/cycl/utils/cfn_test.py +++ /dev/null @@ -1,165 +0,0 @@ -from unittest.mock import Mock, call, patch - -import pytest -from botocore.exceptions import ClientError - -import cycl.utils.cfn as cfn_module -from cycl.utils.cfn import get_all_exports, get_all_imports, parse_name_from_id - - -@pytest.fixture -def cfn_client_mock(): - return Mock(name='cfn_client_mock') - - -@pytest.fixture -def mock_boto3(cfn_client_mock): - with patch.object(cfn_module, 'boto3') as mock: - - def client_side_effect_func(service, **_kwargs): - if service == 'cloudformation': - return cfn_client_mock - return None - - mock.client.side_effect = client_side_effect_func - yield mock - - -@pytest.mark.parametrize( - ('stack_id', 'expected'), - [ - ('arn:aws:cloudformation:us-east-1:000000000000:stack/template-1/05a85f80', 'template-1'), - ('/', ''), - ('', ''), - ], -) -def test_parse_name_from_id(stack_id, expected): - actual = parse_name_from_id(stack_id) - assert actual == expected - - -@pytest.mark.parametrize( - 'expected_exports', - [ - [{'ExportingStackId': 'some-exporting-stack-id', 'Name': 'some-name', 'Value': 'some-value'}], - [], - ], -) -def test_get_all_exports_returns_an_export(expected_exports, mock_boto3, cfn_client_mock): - cfn_client_mock.list_exports.return_value = {'Exports': expected_exports} - - actual_exports = get_all_exports() - - mock_boto3.client.assert_called_once_with('cloudformation') - assert actual_exports == expected_exports - - -def test_get_all_exports_conditionally_creates_client(mock_boto3, cfn_client_mock): - expected_exports = [] - cfn_client_mock.list_exports.return_value = {'Exports': expected_exports} - - actual_exports = get_all_exports(cfn_client=cfn_client_mock) - - mock_boto3.client.assert_not_called() - assert actual_exports == expected_exports - - -def test_get_all_exports_uses_next_token(mock_boto3, cfn_client_mock): - export1 = {'ExportingStackId': 'some-exporting-stack-id-1', 'Name': 'some-name-1', 'Value': 'some-value-1'} - export2 = {'ExportingStackId': 'some-exporting-stack-id-2', 'Name': 'some-name-2', 'Value': 'some-value-2'} - expected_exports = [export1, export2] - cfn_client_mock.list_exports.side_effect = [ - {'Exports': [export1], 'NextToken': 'some-token'}, - {'Exports': [export2]}, - ] - - actual_exports = get_all_exports() - - mock_boto3.client.assert_called_once_with('cloudformation') - cfn_client_mock.list_exports.assert_has_calls( - [ - call(), - call(NextToken='some-token'), - ], - ) - assert cfn_client_mock.list_exports.call_count == 2 - assert actual_exports == expected_exports - - -@pytest.mark.parametrize( - 'expected_imports', - [ - ['some-import-stack-name'], - [], - ], -) -def test_get_all_imports_returns_imports(expected_imports, mock_boto3, cfn_client_mock): - export_name = 'some-export_name' - cfn_client_mock.list_imports.return_value = {'Imports': expected_imports} - - actual_imports = get_all_imports(export_name=export_name) - - mock_boto3.client.assert_called_once_with('cloudformation') - cfn_client_mock.list_imports.assert_called_once_with(ExportName=export_name) - assert actual_imports == expected_imports - - -def test_get_all_imports_conditionally_creates_client(mock_boto3, cfn_client_mock): - export_name = 'some-export_name' - expected_imports = [] - cfn_client_mock.list_imports.return_value = {'Imports': expected_imports} - - actual_imports = get_all_imports(export_name=export_name, cfn_client=cfn_client_mock) - - mock_boto3.client.assert_not_called() - assert actual_imports == expected_imports - - -def test_get_all_imports_uses_next_token(mock_boto3, cfn_client_mock): - export_name = 'some-export_name' - import1 = 'some-import-stack-name-1' - import2 = 'some-import-stack-name-2' - expected_imports = [import1, import2] - cfn_client_mock.list_imports.side_effect = [ - {'Imports': [import1], 'NextToken': 'some-token'}, - {'Imports': [import2]}, - ] - - actual_imports = get_all_imports(export_name=export_name) - - mock_boto3.client.assert_called_once_with('cloudformation') - cfn_client_mock.list_imports.assert_has_calls( - [ - call(ExportName=export_name), - call(ExportName=export_name, NextToken='some-token'), - ], - ) - assert cfn_client_mock.list_imports.call_count == 2 - assert actual_imports == expected_imports - - -def test_get_all_imports_excepts_client_error(mock_boto3, cfn_client_mock): - export_name = 'some-export_name' - # todo: determine what this error message actually looks like - cfn_client_mock.list_imports.side_effect = ClientError( - {'Error': {'Code': 'ValidationError', 'Message': f"Export '{export_name}' is not imported by any stack."}}, - 'ListImports', - ) - - actual_imports = get_all_imports(export_name=export_name) - - mock_boto3.client.assert_called_once_with('cloudformation') - assert actual_imports == [] - - -def test_get_all_imports_raises_client_error(mock_boto3, cfn_client_mock): - export_name = 'some-export_name' - cfn_client_mock.list_imports.side_effect = ClientError( - {'Error': {'Code': 'SomeErrorCode', 'Message': 'is not '}}, - 'some_operation', - ) - - with pytest.raises(ClientError): - get_all_imports(export_name=export_name) - - mock_boto3.client.assert_called_once_with('cloudformation') diff --git a/tests/cycl/utils/log_config_test.py b/tests/cycl/utils/log_config_test.py deleted file mode 100644 index 3607fa8..0000000 --- a/tests/cycl/utils/log_config_test.py +++ /dev/null @@ -1,17 +0,0 @@ -import logging - -from cycl.utils.log_config import configure_log - - -def test_logging_format(caplog): - configure_log(logging.INFO) - - with caplog.at_level(logging.INFO): - logging.info('Test log message') - - assert len(caplog.records) > 0 - - log_message = caplog.text - assert 'INFO' in log_message - assert 'log_config_test.py' in log_message - assert 'Test log message' in log_message From c2af25a3113d82e4b49c112d68fe49181e091119 Mon Sep 17 00:00:00 2001 From: tcm5343 <48961675+tcm5343@users.noreply.github.com> Date: Fri, 7 Feb 2025 20:04:18 -0600 Subject: [PATCH 3/9] fix workflow --- .github/workflows/ci.yml | 4 +- tests/cli_test.py | 88 ++++++++++++++++ tests/cycl_test.py | 187 +++++++++++++++++++++++++++++++++ tests/utils/__init__.py | 0 tests/utils/cfn_test.py | 165 +++++++++++++++++++++++++++++ tests/utils/log_config_test.py | 17 +++ 6 files changed, 460 insertions(+), 1 deletion(-) create mode 100644 tests/cli_test.py create mode 100644 tests/cycl_test.py create mode 100644 tests/utils/__init__.py create mode 100644 tests/utils/cfn_test.py create mode 100644 tests/utils/log_config_test.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2fc85e8..497d6d8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,7 +37,9 @@ jobs: run: ruff format --check - name: test - run: pytest --cov=./src/ ./tests/ + run: | + pip install --editable . + pytest --cov=./src/ ./tests/ - name: coverage run: coverage report diff --git a/tests/cli_test.py b/tests/cli_test.py new file mode 100644 index 0000000..ed7ca08 --- /dev/null +++ b/tests/cli_test.py @@ -0,0 +1,88 @@ +import logging +import sys +from unittest.mock import patch + +import networkx as nx +import pytest + +import cycl.cli as cli_module +from cycl.cli import app + + +@pytest.fixture(autouse=True) +def mock_build_build_dependency_graph(): + with patch.object(cli_module, 'build_dependency_graph') as mock: + mock.return_value = nx.MultiDiGraph() + yield mock + + +@pytest.fixture(autouse=True) +def mock_configure_log(): + with patch.object(cli_module, 'configure_log') as mock: + yield mock + + +def test_app_check_acyclic(): + sys.argv = ['cycl', 'check'] + with pytest.raises(SystemExit) as err: + app() + + assert err.value.code == 0 + + +def test_app_check_cyclic(capsys, mock_build_build_dependency_graph): + graph = nx.MultiDiGraph() + graph.add_edges_from( + [ + (1, 2), + (2, 1), + ] + ) + mock_build_build_dependency_graph.return_value = graph + sys.argv = ['cycl', 'check'] + + with pytest.raises(SystemExit) as err: + app() + + assert err.value.code == 1 + console_output = capsys.readouterr().out + assert 'Cycle found between nodes: [1, 2]' in console_output + + +def test_app_check_cyclic_exit_zero(capsys, mock_build_build_dependency_graph): + graph = nx.MultiDiGraph() + graph.add_edges_from( + [ + (1, 2), + (2, 1), + ] + ) + mock_build_build_dependency_graph.return_value = graph + sys.argv = ['cycl', 'check', '--exit-zero'] + + with pytest.raises(SystemExit) as err: + app() + + assert err.value.code == 0 + console_output = capsys.readouterr().out + assert 'Cycle found between nodes: [1, 2]' in console_output + + +@pytest.mark.parametrize( + ('arg_value', 'log_level'), + [ + ('CRITICAL', logging.CRITICAL), + ('DEBUG', logging.DEBUG), + ('ERROR', logging.ERROR), + ('INFO', logging.INFO), + ('WARNING', logging.WARNING), + ], +) +def test_app_check_acyclic_log_level(mock_configure_log, arg_value, log_level): + sys.argv = ['cycl', 'check', '--log-level', arg_value] + + with pytest.raises(SystemExit) as err: + app() + + assert err.value.code == 0 + mock_configure_log.assert_called_with(log_level) diff --git a/tests/cycl_test.py b/tests/cycl_test.py new file mode 100644 index 0000000..7a489a6 --- /dev/null +++ b/tests/cycl_test.py @@ -0,0 +1,187 @@ +from unittest.mock import patch + +import networkx as nx +import pytest + +import cycl.cycl as cycl_module +from cycl.cycl import build_dependency_graph + + +@pytest.fixture(autouse=True) +def mock_parse_name_from_id(): + with patch.object(cycl_module, 'parse_name_from_id') as mock: + mock.side_effect = lambda x: x + yield mock + + +@pytest.fixture(autouse=True) +def mock_get_all_exports(): + with patch.object(cycl_module, 'get_all_exports') as mock: + mock.return_value = [] + yield mock + + +@pytest.fixture(autouse=True) +def mock_get_all_imports(): + with patch.object(cycl_module, 'get_all_imports') as mock: + mock.return_value = [] + yield mock + + +def test_build_dependency_graph_returns_empty_graph(): + actual_graph = build_dependency_graph() + + assert nx.number_of_nodes(actual_graph) == 0 + assert nx.is_directed_acyclic_graph(actual_graph) + assert next(nx.simple_cycles(actual_graph), []) == [] + + +def test_build_dependency_graph_returns_graph(mock_get_all_exports, mock_get_all_imports, subtests): + """ + Visual representation of expected output graph: + some-exporting-stack-id-1 + │ + ├──► some-importing-stack-name-1 + │ + ├──► some-importing-stack-name-2 + """ + mock_get_all_exports.return_value = [ + { + 'ExportingStackId': 'some-exporting-stack-id-1', + 'Name': 'some-name-1', + 'Value': 'some-value-1', + } + ] + mock_get_all_imports.return_value = [ + 'some-importing-stack-name-1', + 'some-importing-stack-name-2', + ] + expected_edges = [ + ('some-exporting-stack-id-1', 'some-importing-stack-name-1'), + ('some-exporting-stack-id-1', 'some-importing-stack-name-2'), + ] + expected_nodes = [ + 'some-exporting-stack-id-1', + 'some-importing-stack-name-1', + 'some-importing-stack-name-2', + ] + + actual_graph = build_dependency_graph() + + for expected_node in expected_nodes: + with subtests.test(msg='assert graph has node', expected_node=expected_node): + assert actual_graph.has_node(expected_node) + assert nx.number_of_nodes(actual_graph) == len(expected_nodes) + + for expected_edge in expected_edges: + with subtests.test(msg='assert graph has edge', expected_edge=expected_edge): + assert actual_graph.has_edge(*expected_edge) + assert nx.number_of_edges(actual_graph) == len(expected_edges) + + assert nx.is_directed_acyclic_graph(actual_graph) + assert next(nx.simple_cycles(actual_graph), []) == [] + + +def test_build_dependency_graph_returns_graph_with_multiple_exports(mock_get_all_exports, mock_get_all_imports, subtests): + """ + Visual representation of expected output graph: + some-exporting-stack-id-1 + │ + ├──► some-importing-stack-name-1 + │ + ├──► some-importing-stack-name-2 + + some-exporting-stack-id-2 + │ + ├──► some-importing-stack-name-1 + + some-exporting-stack-id-3 (No outgoing edges) + some-importing-stack-name-1 (No outgoing edges) + some-importing-stack-name-2 (No outgoing edges) + """ + mock_get_all_exports.return_value = [ + { + 'ExportingStackId': 'some-exporting-stack-id-1', + 'Name': 'some-name-1', + 'Value': 'some-value-1', + }, + { + 'ExportingStackId': 'some-exporting-stack-id-2', + 'Name': 'some-name-2', + 'Value': 'some-value-2', + }, + ] + + def mock_get_all_imports_side_effect_func(export_name): + if export_name == 'some-name-1': + return [ + 'some-importing-stack-name-1', + 'some-importing-stack-name-2', + ] + if export_name == 'some-name-2': + return [ + 'some-importing-stack-name-1', + ] + return [] # handles export with no import + + mock_get_all_imports.side_effect = mock_get_all_imports_side_effect_func + expected_edges = [ + ('some-exporting-stack-id-1', 'some-importing-stack-name-1'), + ('some-exporting-stack-id-1', 'some-importing-stack-name-2'), + ('some-exporting-stack-id-2', 'some-importing-stack-name-1'), + ] + expected_nodes = [ + 'some-exporting-stack-id-1', + 'some-exporting-stack-id-2', + 'some-importing-stack-name-1', + 'some-importing-stack-name-2', + ] + + actual_graph = build_dependency_graph() + + for expected_node in expected_nodes: + with subtests.test(msg='assert graph has node', expected_node=expected_node): + assert actual_graph.has_node(expected_node) + assert nx.number_of_nodes(actual_graph) == len(expected_nodes) + + for expected_edge in expected_edges: + with subtests.test(msg='assert graph has edge', expected_edge=expected_edge): + assert actual_graph.has_edge(*expected_edge) + assert nx.number_of_edges(actual_graph) == len(expected_edges) + + assert nx.is_directed_acyclic_graph(actual_graph) + assert next(nx.simple_cycles(actual_graph), []) == [] + + +def test_build_dependency_graph_returns_graph_when_export_has_no_imports( + mock_get_all_exports, mock_get_all_imports, subtests +): + """ + Visual representation of expected output graph: + some-exporting-stack-id-1 (No outgoing edges) + """ + mock_get_all_exports.return_value = [ + { + 'ExportingStackId': 'some-exporting-stack-id-1', + 'Name': 'some-name-1', + 'Value': 'some-value-1', + }, + ] + mock_get_all_imports.return_value = [] + expected_edges = [] + expected_nodes = ['some-exporting-stack-id-1'] + + actual_graph = build_dependency_graph() + + for expected_node in expected_nodes: + with subtests.test(msg='assert graph has node', expected_node=expected_node): + assert actual_graph.has_node(expected_node) + assert nx.number_of_nodes(actual_graph) == len(expected_nodes) + + for expected_edge in expected_edges: + with subtests.test(msg='assert graph has edge', expected_edge=expected_edge): + assert actual_graph.has_edge(*expected_edge) + assert nx.number_of_edges(actual_graph) == len(expected_edges) + + assert nx.is_directed_acyclic_graph(actual_graph) + assert next(nx.simple_cycles(actual_graph), []) == [] diff --git a/tests/utils/__init__.py b/tests/utils/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/utils/cfn_test.py b/tests/utils/cfn_test.py new file mode 100644 index 0000000..28244a9 --- /dev/null +++ b/tests/utils/cfn_test.py @@ -0,0 +1,165 @@ +from unittest.mock import Mock, call, patch + +import pytest +from botocore.exceptions import ClientError + +import cycl.utils.cfn as cfn_module +from cycl.utils.cfn import get_all_exports, get_all_imports, parse_name_from_id + + +@pytest.fixture +def cfn_client_mock(): + return Mock(name='cfn_client_mock') + + +@pytest.fixture +def mock_boto3(cfn_client_mock): + with patch.object(cfn_module, 'boto3') as mock: + + def client_side_effect_func(service, **_kwargs): + if service == 'cloudformation': + return cfn_client_mock + return None + + mock.client.side_effect = client_side_effect_func + yield mock + + +@pytest.mark.parametrize( + ('stack_id', 'expected'), + [ + ('arn:aws:cloudformation:us-east-1:000000000000:stack/template-1/05a85f80', 'template-1'), + ('/', ''), + ('', ''), + ], +) +def test_parse_name_from_id(stack_id, expected): + actual = parse_name_from_id(stack_id) + assert actual == expected + + +@pytest.mark.parametrize( + 'expected_exports', + [ + [{'ExportingStackId': 'some-exporting-stack-id', 'Name': 'some-name', 'Value': 'some-value'}], + [], + ], +) +def test_get_all_exports_returns_an_export(expected_exports, mock_boto3, cfn_client_mock): + cfn_client_mock.list_exports.return_value = {'Exports': expected_exports} + + actual_exports = get_all_exports() + + mock_boto3.client.assert_called_once_with('cloudformation') + assert actual_exports == expected_exports + + +def test_get_all_exports_conditionally_creates_client(mock_boto3, cfn_client_mock): + expected_exports = [] + cfn_client_mock.list_exports.return_value = {'Exports': expected_exports} + + actual_exports = get_all_exports(cfn_client=cfn_client_mock) + + mock_boto3.client.assert_not_called() + assert actual_exports == expected_exports + + +def test_get_all_exports_uses_next_token(mock_boto3, cfn_client_mock): + export1 = {'ExportingStackId': 'some-exporting-stack-id-1', 'Name': 'some-name-1', 'Value': 'some-value-1'} + export2 = {'ExportingStackId': 'some-exporting-stack-id-2', 'Name': 'some-name-2', 'Value': 'some-value-2'} + expected_exports = [export1, export2] + cfn_client_mock.list_exports.side_effect = [ + {'Exports': [export1], 'NextToken': 'some-token'}, + {'Exports': [export2]}, + ] + + actual_exports = get_all_exports() + + mock_boto3.client.assert_called_once_with('cloudformation') + cfn_client_mock.list_exports.assert_has_calls( + [ + call(), + call(NextToken='some-token'), + ], + ) + assert cfn_client_mock.list_exports.call_count == 2 + assert actual_exports == expected_exports + + +@pytest.mark.parametrize( + 'expected_imports', + [ + ['some-import-stack-name'], + [], + ], +) +def test_get_all_imports_returns_imports(expected_imports, mock_boto3, cfn_client_mock): + export_name = 'some-export_name' + cfn_client_mock.list_imports.return_value = {'Imports': expected_imports} + + actual_imports = get_all_imports(export_name=export_name) + + mock_boto3.client.assert_called_once_with('cloudformation') + cfn_client_mock.list_imports.assert_called_once_with(ExportName=export_name) + assert actual_imports == expected_imports + + +def test_get_all_imports_conditionally_creates_client(mock_boto3, cfn_client_mock): + export_name = 'some-export_name' + expected_imports = [] + cfn_client_mock.list_imports.return_value = {'Imports': expected_imports} + + actual_imports = get_all_imports(export_name=export_name, cfn_client=cfn_client_mock) + + mock_boto3.client.assert_not_called() + assert actual_imports == expected_imports + + +def test_get_all_imports_uses_next_token(mock_boto3, cfn_client_mock): + export_name = 'some-export_name' + import1 = 'some-import-stack-name-1' + import2 = 'some-import-stack-name-2' + expected_imports = [import1, import2] + cfn_client_mock.list_imports.side_effect = [ + {'Imports': [import1], 'NextToken': 'some-token'}, + {'Imports': [import2]}, + ] + + actual_imports = get_all_imports(export_name=export_name) + + mock_boto3.client.assert_called_once_with('cloudformation') + cfn_client_mock.list_imports.assert_has_calls( + [ + call(ExportName=export_name), + call(ExportName=export_name, NextToken='some-token'), + ], + ) + assert cfn_client_mock.list_imports.call_count == 2 + assert actual_imports == expected_imports + + +def test_get_all_imports_excepts_client_error(mock_boto3, cfn_client_mock): + export_name = 'some-export_name' + # todo: determine what this error message actually looks like + cfn_client_mock.list_imports.side_effect = ClientError( + {'Error': {'Code': 'ValidationError', 'Message': f"Export '{export_name}' is not imported by any stack."}}, + 'ListImports', + ) + + actual_imports = get_all_imports(export_name=export_name) + + mock_boto3.client.assert_called_once_with('cloudformation') + assert actual_imports == [] + + +def test_get_all_imports_raises_client_error(mock_boto3, cfn_client_mock): + export_name = 'some-export_name' + cfn_client_mock.list_imports.side_effect = ClientError( + {'Error': {'Code': 'SomeErrorCode', 'Message': 'is not '}}, + 'some_operation', + ) + + with pytest.raises(ClientError): + get_all_imports(export_name=export_name) + + mock_boto3.client.assert_called_once_with('cloudformation') diff --git a/tests/utils/log_config_test.py b/tests/utils/log_config_test.py new file mode 100644 index 0000000..3607fa8 --- /dev/null +++ b/tests/utils/log_config_test.py @@ -0,0 +1,17 @@ +import logging + +from cycl.utils.log_config import configure_log + + +def test_logging_format(caplog): + configure_log(logging.INFO) + + with caplog.at_level(logging.INFO): + logging.info('Test log message') + + assert len(caplog.records) > 0 + + log_message = caplog.text + assert 'INFO' in log_message + assert 'log_config_test.py' in log_message + assert 'Test log message' in log_message From 26cd4cc2062a341521193d2fca8c75c8ce1d63a6 Mon Sep 17 00:00:00 2001 From: tcm5343 <48961675+tcm5343@users.noreply.github.com> Date: Fri, 7 Feb 2025 20:10:31 -0600 Subject: [PATCH 4/9] use tox in pipeline --- .github/workflows/ci.yml | 17 +++++------------ requirements-dev.txt | 1 + 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 497d6d8..30ecf3b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,21 +14,19 @@ jobs: strategy: fail-fast: false matrix: - os: - - 'ubuntu-latest' - - 'windows-latest' - - 'mac-latest' + os: ['ubuntu-latest', 'windows-latest'] + python-version: ['3.8', '3.9', '3.10', '3.11', '3.12', '3.13'] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 with: - python-version: '3.13' + python-version: ${{ matrix.python-version }} cache: 'pip' - name: install deps - run: pip install -r requirements.txt -r requirements-dev.txt + run: pip install -r requirements-dev.txt - name: lint run: ruff check @@ -37,9 +35,4 @@ jobs: run: ruff format --check - name: test - run: | - pip install --editable . - pytest --cov=./src/ ./tests/ - - - name: coverage - run: coverage report + run: tox -e py diff --git a/requirements-dev.txt b/requirements-dev.txt index bb70b73..4023306 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -3,3 +3,4 @@ pytest-subtests==0.14.1 pytest-sugar==1.0.0 pytest==8.3.4 ruff==0.9.4 +tox==4.24.1 From 606014ee80ede9f13114d9ff5a6adf217e6a6b10 Mon Sep 17 00:00:00 2001 From: tcm5343 <48961675+tcm5343@users.noreply.github.com> Date: Fri, 7 Feb 2025 20:39:54 -0600 Subject: [PATCH 5/9] remove toml support --- pyproject.toml | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 240c9b8..75c0d38 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,19 +84,19 @@ omit = [ log_cli_level = "INFO" addopts = ['--import-mode=importlib'] -[tool.tox] -requires = ["tox>=4.19"] -setenv = 'VIRTUALENV_DISCOVERY=pyenv' -env_list = [ - "3.8", - "3.9", - "3.10", - "3.11", - "3.12", - "3.13", -] +# [tool.tox] +# requires = ["tox>=4.19"] +# setenv = 'VIRTUALENV_DISCOVERY=pyenv' +# env_list = [ +# "3.8", +# "3.9", +# "3.10", +# "3.11", +# "3.12", +# "3.13", +# ] -[tool.tox.env_run_base] -allowlist_externals = ['pytest'] -description = "Run test under {base_python}" -commands = [["pytest", '--cov=./src/', './tests/']] +# [tool.tox.env_run_base] +# allowlist_externals = ['pytest'] +# description = "Run test under {base_python}" +# commands = [["pytest", '--cov=./src/', './tests/']] From ddf3de419a886ede4df2b25a5091e0cf15d8b9b5 Mon Sep 17 00:00:00 2001 From: tcm5343 <48961675+tcm5343@users.noreply.github.com> Date: Fri, 7 Feb 2025 20:44:31 -0600 Subject: [PATCH 6/9] fix pipeline --- .github/workflows/ci.yml | 10 ++++++---- Makefile | 5 +++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 30ecf3b..64f9cb1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,8 +14,8 @@ jobs: strategy: fail-fast: false matrix: - os: ['ubuntu-latest', 'windows-latest'] - python-version: ['3.8', '3.9', '3.10', '3.11', '3.12', '3.13'] + os: ['ubuntu-latest'] + python-version: ['3.10', '3.11', '3.12', '3.13'] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 @@ -26,7 +26,9 @@ jobs: cache: 'pip' - name: install deps - run: pip install -r requirements-dev.txt + run: | + pip install -r requirements-dev.txt + pip install --editable . - name: lint run: ruff check @@ -35,4 +37,4 @@ jobs: run: ruff format --check - name: test - run: tox -e py + run: pytest --cov=./src/ ./tests/ diff --git a/Makefile b/Makefile index deddf34..1052911 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ $(REQ_HASH): $(REQ_FILES) stat -c %Y $(REQ_FILES) | md5sum | awk '{print $$1}' > $(REQ_HASH) @echo "Installing dependencies..." python3 -m venv ./.venv - pip install -e .[dev] + pip install -e . -r requirements-dev.txt install-deps: $(REQ_HASH) ## Install dependencies if requirements files changed @@ -36,5 +36,6 @@ clean: ## Clean generated project files rm -f .coverage rm -rf ./.ruff_cache rm -rf ./.pytest_cache - # rm -rf ./.venv + rm -rf ./.venv + rm -rf ./.tox find . -type d -name "__pycache__" -exec rm -r {} + From 68231230134fd22eaea73ca07f97404cc1b0258d Mon Sep 17 00:00:00 2001 From: tcm5343 <48961675+tcm5343@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:53:20 -0600 Subject: [PATCH 7/9] final clean up for pr --- README.md | 28 ++++++-- TODO.md | 20 ++++-- pyproject.toml | 19 +++--- requirements.txt | 3 +- src/cycl/cli.py | 18 ++++-- src/cycl/cycl.py | 20 +++++- src/cycl/utils/cdk.py | 91 ++++++++++++++++++++++++++ src/cycl/utils/cfn.py | 6 ++ tests/cli_test.py | 24 ++++++- tests/cycl_test.py | 137 +++++++++++++++++++++++++++++++++++----- tests/utils/cdk_test.py | 134 +++++++++++++++++++++++++++++++++++++++ 11 files changed, 455 insertions(+), 45 deletions(-) create mode 100644 src/cycl/utils/cdk.py create mode 100644 tests/utils/cdk_test.py diff --git a/README.md b/README.md index aceb694..68e7c38 100644 --- a/README.md +++ b/README.md @@ -6,11 +6,31 @@ cycl is a CLI and Python SDK to help identify cross-stack import/export circular dependencies, for a given AWS account and region. -### Why use cycl? +## Getting started -Over the lifetime of a project, circular references are bound to be introduced. They may not be noticed until you need to re-deploy some infrastructure. A good example is disaster recovery testing and potentially deploying all your infrastructure from scratch in a new region. This tool allows you to scan +Install `cycl` by running `pip install cycl`. -## Getting Started +### CLI -The project is intended to be ran and published via a +- `cycl check --exit-zero` - exit 0 regardless of result +- `cycl check --log-level` - set the logging level (default: WARNING) +- `cycl check --cdk-out /path/to/cdk.out` - path to cdk.out, where stacks are CDK synthesized to CFN templates +### SDK + +... + +## How to use cycl? + +There are two main use cases for `cycl`. + +1. In a pipeline. `cycl` is best used to detect circular dependencies before a deployment. If you're using the AWS CDK v2 (v1 support coming soon), simply synthesize you templates to a directory and pass that directory to `cycl` using `--cdk-out-path some-path-here `. This allows `cycl` to find all existing cycles and then those to be introduced by the deployment. This prevents the circular dependency from ever being introduced. If your pipeline deploys more than once, you should execute `cycl` before each deployment. +2. To perform analysis. While a CLI is best used in a pipeline, if you require analysis which is not currently supported, you can use the SDK. The SDK gives you all the information that `cycl` collects. + +## Why use cycl? + +Over the lifetime of a project, circular references are bound to be introduced. They may not be noticed until you need to re-deploy some infrastructure. A good example is disaster recovery testing and potentially deploying all your infrastructure from scratch in a new region. This tool detects those changes. + +## Contributing + +`cycl` is being actively developed, instructions to come as it becomes more stable. diff --git a/TODO.md b/TODO.md index 0f950d5..9441cb8 100644 --- a/TODO.md +++ b/TODO.md @@ -1,14 +1,20 @@ # TODO.md +- output the topological generations of the graph + - `cycl check --generations` +- ignoring known cycles, [this](https://cs.stackexchange.com/questions/90481/how-to-remove-cycles-from-a-directed-graph) may help + - `cycl check --ignore-cycle-contains` + - `cycl check --ignore-cycle` +- reducing the stacks, for example, a tag on a stack representing the github repo name + - `cycl check --reduce-dependencies-on` + - `cycl check --reduce-generations-on` + - [ ] [fully configure dependabot](https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions) - [ ] configure make file - [ ] dprint for other file formatting - [ ] automatic release versioning for pypi, run unit tests against package? Is this possible? -- [ ] isort - [ ] configuration file using cli, use toml, custom rc file, env vars? - - -Next steps: -1. Add `cycl --check` as first feature, publish v1.0.0 to pypi -2. Add ability to ignore known cycles, how to break the cycle? -3. Add ability to reduce via tags, check out jquery --reduce-on maybe? +- [ ] CDK v1 support +- [ ] Test with stages that the correct manifest is analyzed +- [ ] What if cdk out synth is for multiple accounts? We may need to determine what account we have credentials for and only analyze those templates +- [ ] automatic documentation generation diff --git a/pyproject.toml b/pyproject.toml index 75c0d38..6169583 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,8 +3,10 @@ requires = ['setuptools>=64', 'setuptools-scm>=8'] build-backend = 'setuptools.build_meta' [project] + name = 'cycl' -version='0.0.2' +version='0.1.1' +requires-python = ">=3.8" description = 'CLI and Python SDK to help identify cross-stack import/export circular dependencies, for a given AWS account and region.' readme = 'README.md' keywords = ['aws', 'cdk', 'cycle', 'circular', 'dependency', 'infrastructure'] @@ -12,6 +14,12 @@ classifiers = [ 'Intended Audience :: Developers', 'Natural Language :: English', 'Programming Language :: Python :: 3', + "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", + "Programming Language :: Python :: 3.13", 'Programming Language :: Python', 'Topic :: Software Development', ] @@ -20,15 +28,6 @@ dependencies = [ 'networkx~=3.0', ] -[project.optional-dependencies] -dev = [ - 'pytest-cov==6.0.0', - 'pytest-subtests==0.14.1', - 'pytest-sugar==1.0.0', - 'pytest==8.3.4', - 'ruff==0.9.4', -] - [project.urls] Repository = 'http://github.com/tcm5343/cycl' diff --git a/requirements.txt b/requirements.txt index 8b13789..f15e98e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1,2 @@ - +boto3==1.36.14 +networkx==3.4.2 diff --git a/src/cycl/cli.py b/src/cycl/cli.py index 9ac51a6..54b4ad5 100644 --- a/src/cycl/cli.py +++ b/src/cycl/cli.py @@ -1,12 +1,16 @@ import argparse import logging +import pathlib import sys +from logging import getLogger import networkx as nx from cycl import build_dependency_graph from cycl.utils.log_config import configure_log +log = getLogger(__name__) + def app() -> None: parser = argparse.ArgumentParser(description='Check for cross-stack import/export circular dependencies.') @@ -17,22 +21,28 @@ def app() -> None: parent_parser.add_argument( '--log-level', choices=['DEBUG', 'INFO', 'WARNING', 'ERROR', 'CRITICAL'], - default='ERROR', - help='Set the logging level (default: ERROR)', + default='WARNING', + help='set the logging level (default: WARNING)', + ) + parent_parser.add_argument( + '--cdk-out', + type=pathlib.Path, + help='path to cdk.out, where stacks are CDK synthesized to CFN templates', ) subparsers.add_parser('check', parents=[parent_parser], help='Check for cycles in AWS stack imports/exports') args = parser.parse_args() configure_log(getattr(logging, args.log_level)) + log.info(args) if args.action == 'check': cycle_found = False - graph = build_dependency_graph() + graph = build_dependency_graph(cdk_out_path=args.cdk_out) cycles = nx.simple_cycles(graph) for cycle in cycles: cycle_found = True - print(f'Cycle found between nodes: {cycle}') + print(f'cycle found between nodes: {cycle}') if cycle_found and not args.exit_zero: sys.exit(1) sys.exit(0) diff --git a/src/cycl/cycl.py b/src/cycl/cycl.py index 291c046..72ac0f4 100644 --- a/src/cycl/cycl.py +++ b/src/cycl/cycl.py @@ -1,19 +1,37 @@ +from __future__ import annotations + from logging import getLogger +from pathlib import Path import networkx as nx +from cycl.utils.cdk import get_cdk_out_imports from cycl.utils.cfn import get_all_exports, get_all_imports, parse_name_from_id log = getLogger(__name__) -def build_dependency_graph() -> nx.MultiDiGraph: +def build_dependency_graph(cdk_out_path: Path | None = None) -> nx.MultiDiGraph: dep_graph = nx.MultiDiGraph() + cdk_out_imports = {} + if cdk_out_path: + cdk_out_imports = get_cdk_out_imports(Path(cdk_out_path)) + exports = get_all_exports() + # this could be made more efficient if get_all_exports returns a dict instead of a list, no need to iterate through + for export_name in cdk_out_imports: + if not any(export_name == export['Name'] for export in exports): + log.warning( + 'found an export (%s) which has not been deployed yet about to be imported stack(s): (%s)', + export_name, + cdk_out_imports[export_name], + ) + for export in exports: export['ExportingStackName'] = parse_name_from_id(export['ExportingStackId']) export['ImportingStackNames'] = get_all_imports(export_name=export['Name']) + export.setdefault('ImportingStackNames', []).extend(cdk_out_imports.get(export['Name'], [])) edges = [ (export['ExportingStackName'], importing_stack_name) for importing_stack_name in export['ImportingStackNames'] ] diff --git a/src/cycl/utils/cdk.py b/src/cycl/utils/cdk.py new file mode 100644 index 0000000..13c4bd7 --- /dev/null +++ b/src/cycl/utils/cdk.py @@ -0,0 +1,91 @@ +from __future__ import annotations + +import json +from os import walk +from pathlib import Path +from logging import getLogger + +log = getLogger(__name__) + + +class InvalidCdkOutPathError(Exception): + def __init__(self, message: str = 'An error occurred') -> None: + super().__init__(message) + + +def __find_import_values(data: dict) -> list[str]: + """recursively search for all 'Fn::ImportValue' keys and return their values""" + results = [] + + if isinstance(data, dict): + for key, value in data.items(): + if key == 'Fn::ImportValue': + results.append(value) + else: + results.extend(__find_import_values(value)) + + elif isinstance(data, list): + for item in data: + results.extend(__find_import_values(item)) + + return results + + +def __get_import_values_from_template(file_path: Path) -> list[str]: + """todo: handle yaml templates too""" + with Path.open(file_path) as f: + json_data = json.load(f) + return __find_import_values(json_data) + + +def __get_stack_name_from_manifest(path_to_manifest: Path, template_file_name: str) -> str: + with Path.open(path_to_manifest) as f: + json_data = json.load(f) + return json_data['artifacts'][template_file_name.split('.')[0]]['displayName'] + + +def __validate_cdk_out_path(cdk_out_path: Path) -> Path: + errors = [] + if Path.exists(cdk_out_path): + if not cdk_out_path.is_dir(): + errors.append('path must be a directory') + else: + errors.append("path doesn't exist") + + # handle if path is where cdk.out/ is or is directly to cdk.out + if Path(cdk_out_path).name != 'cdk.out': + cdk_out_path = Path(cdk_out_path) / 'cdk.out' + + if not Path.exists(Path(cdk_out_path) / 'cdk.out'): + errors.append('unable to find CDK stack synthesis output in provided directory, did you synth?') + + if errors: + errors_formatted = '\n\t - '.join(errors) + error_message = f'Invalid path provided for --cdk-out {cdk_out_path}:\n\t - {errors_formatted}' + raise InvalidCdkOutPathError(error_message) + + return cdk_out_path + + +def get_cdk_out_imports(cdk_out_path: Path) -> dict[str, list[str]]: + """ + map an export name to a list of stacks which import it + + function does not take into consideration exports + - if we found an export, we may not be able to resolve the name of it + - AWS has built in circular dependency detection inside of a stack + - a circular dependency couldn't be introduced in a single deployment + """ + cdk_out_path = __validate_cdk_out_path(cdk_out_path) + + stack_import_mapping = {} + for root, _dirs, files in walk(cdk_out_path): + for file in files: + if file.endswith('template.json'): + imported_export_names = __get_import_values_from_template(Path(root) / file) + if imported_export_names: + manifest_path = Path(root) / 'manifest.json' + stack_name = __get_stack_name_from_manifest(manifest_path, file) + for export_name in imported_export_names: + stack_import_mapping.setdefault(export_name, []).append(stack_name) + return stack_import_mapping diff --git a/src/cycl/utils/cfn.py b/src/cycl/utils/cfn.py index 5b31cfa..1497b4d 100644 --- a/src/cycl/utils/cfn.py +++ b/src/cycl/utils/cfn.py @@ -26,10 +26,13 @@ def get_all_exports(cfn_client: BaseClient | None = None) -> list[dict]: exports = [] resp = cfn_client.list_exports() + log.debug(resp) exports.extend(resp['Exports']) while token := resp.get('NextToken'): resp = cfn_client.list_exports(NextToken=token) + log.debug(resp) exports.extend(resp['Exports']) + log.debug(exports) return exports @@ -40,12 +43,15 @@ def get_all_imports(export_name: str, cfn_client: BaseClient | None = None) -> l imports = [] try: resp = cfn_client.list_imports(ExportName=export_name) + log.debug(resp) imports.extend(resp['Imports']) while token := resp.get('NextToken'): resp = cfn_client.list_imports(ExportName=export_name, NextToken=token) + log.debug(resp) imports.extend(resp['Imports']) except ClientError as err: if 'is not imported by any stack' not in repr(err): raise log.debug('') + log.debug(imports) return imports diff --git a/tests/cli_test.py b/tests/cli_test.py index ed7ca08..0634c72 100644 --- a/tests/cli_test.py +++ b/tests/cli_test.py @@ -22,6 +22,26 @@ def mock_configure_log(): yield mock +def test_app_no_action(capsys): + sys.argv = ['cycl'] + with pytest.raises(SystemExit) as err: + app() + + assert err.value.code == 2 + console_output = capsys.readouterr().err + assert 'cycl: error: the following arguments are required: action' in console_output + + +def test_app_unsupported_action(capsys): + sys.argv = ['cycl', 'something'] + with pytest.raises(SystemExit) as err: + app() + + assert err.value.code == 2 + console_output = capsys.readouterr().err + assert 'cycl: error: argument action: invalid choice: \'something\'' in console_output + + def test_app_check_acyclic(): sys.argv = ['cycl', 'check'] with pytest.raises(SystemExit) as err: @@ -46,7 +66,7 @@ def test_app_check_cyclic(capsys, mock_build_build_dependency_graph): assert err.value.code == 1 console_output = capsys.readouterr().out - assert 'Cycle found between nodes: [1, 2]' in console_output + assert 'cycle found between nodes: [1, 2]' in console_output def test_app_check_cyclic_exit_zero(capsys, mock_build_build_dependency_graph): @@ -65,7 +85,7 @@ def test_app_check_cyclic_exit_zero(capsys, mock_build_build_dependency_graph): assert err.value.code == 0 console_output = capsys.readouterr().out - assert 'Cycle found between nodes: [1, 2]' in console_output + assert 'cycle found between nodes: [1, 2]' in console_output @pytest.mark.parametrize( diff --git a/tests/cycl_test.py b/tests/cycl_test.py index 7a489a6..9d6a7f5 100644 --- a/tests/cycl_test.py +++ b/tests/cycl_test.py @@ -1,3 +1,4 @@ +from pathlib import Path from unittest.mock import patch import networkx as nx @@ -10,7 +11,7 @@ @pytest.fixture(autouse=True) def mock_parse_name_from_id(): with patch.object(cycl_module, 'parse_name_from_id') as mock: - mock.side_effect = lambda x: x + mock.side_effect = lambda x: f'{x}-stack-name' yield mock @@ -28,6 +29,13 @@ def mock_get_all_imports(): yield mock +@pytest.fixture(autouse=True) +def mock_get_cdk_out_imports(): + with patch.object(cycl_module, 'get_cdk_out_imports') as mock: + mock.return_value = {} + yield mock + + def test_build_dependency_graph_returns_empty_graph(): actual_graph = build_dependency_graph() @@ -36,10 +44,12 @@ def test_build_dependency_graph_returns_empty_graph(): assert next(nx.simple_cycles(actual_graph), []) == [] -def test_build_dependency_graph_returns_graph(mock_get_all_exports, mock_get_all_imports, subtests): +def test_build_dependency_graph_returns_graph( + mock_get_all_exports, mock_get_all_imports, subtests, mock_get_cdk_out_imports +): """ Visual representation of expected output graph: - some-exporting-stack-id-1 + some-exporting-stack-id-1-stack-name │ ├──► some-importing-stack-name-1 │ @@ -57,17 +67,18 @@ def test_build_dependency_graph_returns_graph(mock_get_all_exports, mock_get_all 'some-importing-stack-name-2', ] expected_edges = [ - ('some-exporting-stack-id-1', 'some-importing-stack-name-1'), - ('some-exporting-stack-id-1', 'some-importing-stack-name-2'), + ('some-exporting-stack-id-1-stack-name', 'some-importing-stack-name-1'), + ('some-exporting-stack-id-1-stack-name', 'some-importing-stack-name-2'), ] expected_nodes = [ - 'some-exporting-stack-id-1', + 'some-exporting-stack-id-1-stack-name', 'some-importing-stack-name-1', 'some-importing-stack-name-2', ] actual_graph = build_dependency_graph() + mock_get_cdk_out_imports.assert_not_called() for expected_node in expected_nodes: with subtests.test(msg='assert graph has node', expected_node=expected_node): assert actual_graph.has_node(expected_node) @@ -85,17 +96,17 @@ def test_build_dependency_graph_returns_graph(mock_get_all_exports, mock_get_all def test_build_dependency_graph_returns_graph_with_multiple_exports(mock_get_all_exports, mock_get_all_imports, subtests): """ Visual representation of expected output graph: - some-exporting-stack-id-1 + some-exporting-stack-id-1-stack-name │ ├──► some-importing-stack-name-1 │ ├──► some-importing-stack-name-2 - some-exporting-stack-id-2 + some-exporting-stack-id-2-stack-name │ ├──► some-importing-stack-name-1 - some-exporting-stack-id-3 (No outgoing edges) + some-exporting-stack-id-3-stack-name (No outgoing edges) some-importing-stack-name-1 (No outgoing edges) some-importing-stack-name-2 (No outgoing edges) """ @@ -126,13 +137,13 @@ def mock_get_all_imports_side_effect_func(export_name): mock_get_all_imports.side_effect = mock_get_all_imports_side_effect_func expected_edges = [ - ('some-exporting-stack-id-1', 'some-importing-stack-name-1'), - ('some-exporting-stack-id-1', 'some-importing-stack-name-2'), - ('some-exporting-stack-id-2', 'some-importing-stack-name-1'), + ('some-exporting-stack-id-1-stack-name', 'some-importing-stack-name-1'), + ('some-exporting-stack-id-1-stack-name', 'some-importing-stack-name-2'), + ('some-exporting-stack-id-2-stack-name', 'some-importing-stack-name-1'), ] expected_nodes = [ - 'some-exporting-stack-id-1', - 'some-exporting-stack-id-2', + 'some-exporting-stack-id-1-stack-name', + 'some-exporting-stack-id-2-stack-name', 'some-importing-stack-name-1', 'some-importing-stack-name-2', ] @@ -158,7 +169,7 @@ def test_build_dependency_graph_returns_graph_when_export_has_no_imports( ): """ Visual representation of expected output graph: - some-exporting-stack-id-1 (No outgoing edges) + some-exporting-stack-id-1-stack-name (No outgoing edges) """ mock_get_all_exports.return_value = [ { @@ -169,7 +180,7 @@ def test_build_dependency_graph_returns_graph_when_export_has_no_imports( ] mock_get_all_imports.return_value = [] expected_edges = [] - expected_nodes = ['some-exporting-stack-id-1'] + expected_nodes = ['some-exporting-stack-id-1-stack-name'] actual_graph = build_dependency_graph() @@ -185,3 +196,97 @@ def test_build_dependency_graph_returns_graph_when_export_has_no_imports( assert nx.is_directed_acyclic_graph(actual_graph) assert next(nx.simple_cycles(actual_graph), []) == [] + + +def test_build_dependency_graph_returns_empty_graph_with_cdk_out_path(mock_get_cdk_out_imports): + actual_graph = build_dependency_graph(cdk_out_path='some-cdk-out-path') + + mock_get_cdk_out_imports.assert_called_once_with(Path('some-cdk-out-path')) + assert nx.number_of_nodes(actual_graph) == 0 + assert nx.is_directed_acyclic_graph(actual_graph) + assert next(nx.simple_cycles(actual_graph), []) == [] + + +def test_build_dependency_graph_returns_graph_with_cdk_out_path( + mock_get_all_exports, mock_get_all_imports, subtests, mock_get_cdk_out_imports +): + """ + Visual representation of expected output graph: + some-exporting-stack-id-1-stack-name + │ + ├──► some-importing-stack-name-1 + │ + ├──► some-importing-stack-name-2 + │ + ├──► some-cdk-out-stack-name-1 + """ + mock_get_cdk_out_imports.return_value = {'some-name-1': ['some-cdk-out-stack-name-1']} + mock_get_all_exports.return_value = [ + { + 'ExportingStackId': 'some-exporting-stack-id-1', + 'Name': 'some-name-1', + 'Value': 'some-value-1', + } + ] + mock_get_all_imports.return_value = [ + 'some-importing-stack-name-1', + 'some-importing-stack-name-2', + ] + expected_edges = [ + ('some-exporting-stack-id-1-stack-name', 'some-importing-stack-name-1'), + ('some-exporting-stack-id-1-stack-name', 'some-importing-stack-name-2'), + ('some-exporting-stack-id-1-stack-name', 'some-cdk-out-stack-name-1'), + ] + expected_nodes = [ + 'some-exporting-stack-id-1-stack-name', + 'some-importing-stack-name-1', + 'some-importing-stack-name-2', + 'some-cdk-out-stack-name-1', + ] + + actual_graph = build_dependency_graph(cdk_out_path='some-cdk-out-path') + + mock_get_cdk_out_imports.assert_called_once_with(Path('some-cdk-out-path')) + for expected_node in expected_nodes: + with subtests.test(msg='assert graph has node', expected_node=expected_node): + assert actual_graph.has_node(expected_node) + assert nx.number_of_nodes(actual_graph) == len(expected_nodes) + + for expected_edge in expected_edges: + with subtests.test(msg='assert graph has edge', expected_edge=expected_edge): + assert actual_graph.has_edge(*expected_edge) + assert nx.number_of_edges(actual_graph) == len(expected_edges) + + assert nx.is_directed_acyclic_graph(actual_graph) + assert next(nx.simple_cycles(actual_graph), []) == [] + + +def test_build_dependency_graph_returns_graph_with_cdk_out_path_and_no_existing_exports( + mock_get_all_exports, mock_get_all_imports, subtests, mock_get_cdk_out_imports +): + """ + If we are deploying for the first time and have two independent stacks: stack1 and stack2. stack1 creates + an export and stack1 imports it, with dependsOn(), this will deploy successfully. We are unable to find + the imported export so it should be safe to ignore in our graph. + """ + mock_get_cdk_out_imports.return_value = {'some-name-1': ['some-cdk-out-stack-name-1']} + mock_get_all_exports.return_value = [] + mock_get_all_imports.return_value = [] + expected_edges = [] + expected_nodes = [] + + actual_graph = build_dependency_graph(cdk_out_path='some-cdk-out-path') + + mock_get_cdk_out_imports.assert_called_once_with(Path('some-cdk-out-path')) + for expected_node in expected_nodes: + with subtests.test(msg='assert graph has node', expected_node=expected_node): + assert actual_graph.has_node(expected_node) + assert nx.number_of_nodes(actual_graph) == len(expected_nodes) + + for expected_edge in expected_edges: + with subtests.test(msg='assert graph has edge', expected_edge=expected_edge): + assert actual_graph.has_edge(*expected_edge) + assert nx.number_of_edges(actual_graph) == len(expected_edges) + + assert nx.is_directed_acyclic_graph(actual_graph) + assert next(nx.simple_cycles(actual_graph), []) == [] diff --git a/tests/utils/cdk_test.py b/tests/utils/cdk_test.py new file mode 100644 index 0000000..81a784c --- /dev/null +++ b/tests/utils/cdk_test.py @@ -0,0 +1,134 @@ +import json +import shutil +from pathlib import Path +from unittest.mock import patch + +import pytest + +import cycl.utils.cdk as cdk_module +from cycl.utils.cdk import InvalidCdkOutPathError, get_cdk_out_imports + + +@pytest.fixture +def cdk_template_mock(): + return { + 'Resources': { + 'MyResource': {'Type': 'AWS::S3::Bucket', 'Properties': {'BucketName': {'Fn::ImportValue': 'MyExportedBucket1'}}} + } + } + + +@pytest.fixture +def cdk_manifest_mock(): + return {'artifacts': {'test-stack-1': {'displayName': 'TestStack1'}}} + + +@pytest.fixture +def cdk_out_mock(tmp_path, cdk_template_mock, cdk_manifest_mock): + cdk_out_path = tmp_path / 'cdk.out' + cdk_out_path.mkdir() + Path(cdk_out_path / 'cdk.out').touch() + + template_path = cdk_out_path / 'test-stack-1.template.json' + with template_path.open('w') as f: + json.dump(cdk_template_mock, f) + + manifest_path = cdk_out_path / 'manifest.json' + with manifest_path.open('w') as f: + json.dump(cdk_manifest_mock, f) + + return cdk_out_path + + +@pytest.fixture +def mock_walk(): + with patch.object(cdk_module, 'walk') as mock: + yield mock + + +def test_get_cdk_out_imports_no_imports(cdk_out_mock, cdk_template_mock): + expected = {} + cdk_template_mock = {} + + template_path = cdk_out_mock / 'test-stack-1.template.json' + with template_path.open('w') as f: + json.dump(cdk_template_mock, f) + + actual = get_cdk_out_imports(cdk_out_mock) + + assert actual == expected + + +def test_get_cdk_out_imports_has_imports(cdk_out_mock): + expected = {'MyExportedBucket1': ['TestStack1']} + actual = get_cdk_out_imports(cdk_out_mock) + assert actual == expected + + +def test_get_cdk_out_imports_has_imports_in_list(cdk_out_mock, cdk_template_mock): + expected = {'MyExportedBucket1': ['TestStack1']} + cdk_template_mock['Resources']['MyResource']['Properties']['BucketName'] = [ + {'Fn::ImportValue': 'MyExportedBucket1'}, + ] + + template_path = cdk_out_mock / 'test-stack-1.template.json' + with template_path.open('w') as f: + json.dump(cdk_template_mock, f) + + actual = get_cdk_out_imports(cdk_out_mock) + assert actual == expected + + +def test_get_cdk_out_raises_error_if_no_cdk_out_file_in_folder(cdk_out_mock): + cdk_out_file = Path(cdk_out_mock / 'cdk.out') + assert cdk_out_file.is_file # to confirm we are removing the file, not directory with the same name + cdk_out_file.unlink(missing_ok=False) + + with pytest.raises( + InvalidCdkOutPathError, match='unable to find CDK stack synthesis output in provided directory, did you synth?' + ): + get_cdk_out_imports(cdk_out_mock) + + +def test_get_cdk_out_raises_error_if_no_cdk_out_folder(cdk_out_mock): + shutil.rmtree(cdk_out_mock) + + with pytest.raises(InvalidCdkOutPathError, match="path doesn't exist"): + get_cdk_out_imports(cdk_out_mock) + + +def test_get_cdk_out_raises_error_if_pointing_to_file(cdk_out_mock): + with pytest.raises(InvalidCdkOutPathError, match='path must be a directory'): + get_cdk_out_imports(cdk_out_mock / 'cdk.out') + + +def test_get_cdk_out_adds_cdk_out_dir_if_not_already_there(cdk_out_mock, tmp_path, mock_walk): + """ + example is infra/ being passed instead of infra/cdk.out, simply append cdk.out/ + """ + get_cdk_out_imports(tmp_path) + mock_walk.assert_called_once_with(cdk_out_mock) + + +def test_get_cdk_out_imports_with_two_stacks(cdk_out_mock, cdk_template_mock, cdk_manifest_mock): + expected = {'MyExportedBucket1': ['TestStack1'], 'MyExportedBucket2': ['TestStack2']} + + cdk_template_mock['Resources']['MyResource']['Properties']['BucketName']['Fn::ImportValue'] = 'MyExportedBucket2' + template_path = cdk_out_mock / 'test-stack-2.template.json' + with template_path.open('w') as f: + json.dump(cdk_template_mock, f) + + cdk_manifest_mock['artifacts']['test-stack-2'] = { + 'displayName': 'TestStack2', + } + manifest_path = cdk_out_mock / 'manifest.json' + with manifest_path.open('w') as f: + json.dump(cdk_manifest_mock, f) + + actual = get_cdk_out_imports(cdk_out_mock) + assert actual == expected + + +@pytest.mark.skip +def test_get_cdk_out_imports_with_stages(): + """TODO: determine what the expected structure is""" From 2c98f687d059e006b6f72db4bf298ea9b324480e Mon Sep 17 00:00:00 2001 From: tcm5343 <48961675+tcm5343@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:55:23 -0600 Subject: [PATCH 8/9] remove tox --- pyproject.toml | 17 ----------------- requirements-dev.txt | 1 - 2 files changed, 18 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 6169583..cd3bde2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,20 +82,3 @@ omit = [ [tool.pytest.ini_options] log_cli_level = "INFO" addopts = ['--import-mode=importlib'] - -# [tool.tox] -# requires = ["tox>=4.19"] -# setenv = 'VIRTUALENV_DISCOVERY=pyenv' -# env_list = [ -# "3.8", -# "3.9", -# "3.10", -# "3.11", -# "3.12", -# "3.13", -# ] - -# [tool.tox.env_run_base] -# allowlist_externals = ['pytest'] -# description = "Run test under {base_python}" -# commands = [["pytest", '--cov=./src/', './tests/']] diff --git a/requirements-dev.txt b/requirements-dev.txt index 4023306..bb70b73 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -3,4 +3,3 @@ pytest-subtests==0.14.1 pytest-sugar==1.0.0 pytest==8.3.4 ruff==0.9.4 -tox==4.24.1 From 467789fb3bb82f30332c28330e7d09a59e4181ec Mon Sep 17 00:00:00 2001 From: tcm5343 <48961675+tcm5343@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:57:28 -0600 Subject: [PATCH 9/9] fix validation --- src/cycl/utils/cdk.py | 2 +- tests/cli_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cycl/utils/cdk.py b/src/cycl/utils/cdk.py index 13c4bd7..a5270f1 100644 --- a/src/cycl/utils/cdk.py +++ b/src/cycl/utils/cdk.py @@ -1,9 +1,9 @@ from __future__ import annotations import json +from logging import getLogger from os import walk from pathlib import Path -from logging import getLogger log = getLogger(__name__) diff --git a/tests/cli_test.py b/tests/cli_test.py index 0634c72..b898a6f 100644 --- a/tests/cli_test.py +++ b/tests/cli_test.py @@ -39,7 +39,7 @@ def test_app_unsupported_action(capsys): assert err.value.code == 2 console_output = capsys.readouterr().err - assert 'cycl: error: argument action: invalid choice: \'something\'' in console_output + assert "cycl: error: argument action: invalid choice: 'something'" in console_output def test_app_check_acyclic():