-
Notifications
You must be signed in to change notification settings - Fork 1
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
Improve Unit Testing #152
Improve Unit Testing #152
Conversation
…ocessors and build
… definitino folder builder tests
)] | ||
|
||
# Testing each individial part of artifact are equal | ||
self.assertEqual(artifact_details[0][0].artifact_name, mock_artifact[0].artifact_name) |
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.
this makes me think a class for the return type of get_artifact_details might be useful. You might even reduce the amount of code by not needing to document the current Tuple(list,list) in each get_artifact_details implementation, and it would make these assertions easier to understand
src/aosm/azext_aosm/tests/latest/unit_test/test_processors/test_core_arm_processor.py
Show resolved
Hide resolved
..._aosm/tests/latest/unit_test/test_definiton_folder_builder/test_definition_folder_builder.py
Show resolved
Hide resolved
read_data='{"$schema": "#", "resources": { } }')) | ||
def test_get_schema_no_parameters(self): | ||
"""Test getting the schema for the ARM template input when no parameters are found.""" | ||
# Assert logger warning when no parameters in file |
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.
is this really the only error case for ArmTemplateInput?
(I'm kind of surprised it's so relaxed about its inputs that the worst thing that could possibly happen results in a warning log)
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.
There is also an error case if template_path does not exist, in which case the open
will raise an error (we which are not handling - bad thing but not really part of your story.)
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.
I didn't see you add the test for this error
src/aosm/azext_aosm/tests/latest/unit_test/test_inputs/test_arm_template_input.py
Show resolved
Hide resolved
…s; improved arm processor testing
have you run style and linting? |
src/aosm/azext_aosm/tests/latest/unit_test/test_inputs/test_arm_template_input.py
Show resolved
Hide resolved
src/aosm/azext_aosm/tests/latest/unit_test/test_inputs/test_arm_template_input.py
Show resolved
Hide resolved
read_data='{"$schema": "#", "resources": { } }')) | ||
def test_get_schema_no_parameters(self): | ||
"""Test getting the schema for the ARM template input when no parameters are found.""" | ||
# Assert logger warning when no parameters in file |
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.
There is also an error case if template_path does not exist, in which case the open
will raise an error (we which are not handling - bad thing but not really part of your story.)
src/aosm/azext_aosm/tests/latest/unit_test/test_inputs/test_helm_chart_input.py
Show resolved
Hide resolved
@@ -82,12 +68,20 @@ def test_get_schema_with_params(self): | |||
'required': [], | |||
'type': 'object' | |||
} | |||
print("SCHEMA", schema) | |||
self.assertEqual(schema, expected_schema) | |||
|
|||
@patch("builtins.open", mock_open( | |||
read_data='{"$schema": "#", "resources": { } }')) |
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.
what about this patch open? We should be able to solve it the same was as the previous one
This PR includes:
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
General Guidelines
azdev style <YOUR_EXT>
locally? (pip install azdev
required)python scripts/ci/test_index.py -q
locally? (pip install wheel==0.30.0
required)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.json
automatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json
.