-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(terraform): add new function to create module and definitions with tests #5362
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work, looking forward to the next steps 🎉
@@ -369,25 +370,19 @@ def parse_multi_graph_hcl_module( | |||
|
|||
def create_definition_by_dirs(self, tf_definitions: dict[TFDefinitionKey, dict[str, list[dict[str, Any]]]] | |||
) -> dict[str, list[dict[TFDefinitionKey, dict[str, Any]]]]: | |||
dirs_to_definitions: dict[str, list[dict[TFDefinitionKey, dict[str, Any]]]] = {} | |||
dirs_to_definitions: dict[str, list[dict[TFDefinitionKey, dict[str, Any]]]] = defaultdict(list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice improvement :)
@@ -108,6 +109,39 @@ def test_set_variables_values_from_modules(self): | |||
else: | |||
self.assertEqual(var_value, default_val) | |||
|
|||
def test_definition_creation_by_dirs(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice test!
However, I still think we need some more for the logic - maybe add 1 with more than 1 created module.
you can find a good example under tests/terraform/graph/resources/modules
.
This one even utilizes different modules with a reference to other directories, so I feel it will be a very strong test.
(You don't have to take the whole folder as it might be also too big, maybe just one sub directory like stacks
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🏖️
…th tests (bridgecrewio#5362) * add new function to create module and definitions with tests * fix according to comments * rename the modules to modules_and_definitions_tuple * fix mypy issue * fix more mypy issue * fix false over identation (according to mypy) * fix more flake8 issues * fix even more mypy issues that appeared due to lint fixes * add new UT and fix according to comments * change regular dict to defaultdict * change tests import * add new test with two modules --------- Co-authored-by: Max Amelchenko <mamelchenko@paloaltonetworks.com>
This PR introduces a new function in order to create a multi graph hcl module, instead of the previous single graph hcl module.
At the moment it is not called from anywhere and will be used in subsequent PRs.
The old function will not be removed and it will stay and be called depending on env var.
Checklist: