Refactoring/Testing with Cody AI from Sourcegraph #55
Conversation
highb
commented
Oct 3, 2023
highb
commented
- Fix image validation so we can show all errors at once
- Extract validation into separate module w/ tests
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Quality Gates: FAILED
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_images
- Complex Method validate.py: validate_charts
- Bumpy Road Ahead validate.py: validate_images
- Bumpy Road Ahead validate.py: validate_charts
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
| def validate_images(images): | ||
| """ | ||
| Validate a list of images to ensure they have the required fields and that the names and newNames are unique. | ||
|
|
||
| Args: | ||
| images (list): The list of images to validate. | ||
|
|
||
| Returns: | ||
| bool: True if all images are valid, False otherwise. | ||
| """ | ||
| originally_dict = False | ||
| # Convert to list if it is a dict (which is the case when we are validating | ||
| # images from a promotion file) | ||
| if isinstance(images, dict): | ||
| originally_dict = True | ||
| images = list(images.values()) | ||
|
|
||
| # Ensure that all image names are unique | ||
| duplicates = find_duplicates(images, "name") | ||
| if len(duplicates) > 0: | ||
| logger.fatal( | ||
| f"Found duplicate image names: {' '.join(duplicates)}. Images must have unique names." | ||
| ) | ||
| sys.exit(1) | ||
|
|
||
| # Ensure that all image newNames are unique | ||
| duplicates = find_duplicates(images, "newName") | ||
| if len(duplicates) > 0: | ||
| logger.fatal( | ||
| f"Found duplicate image newNames: {' '.join(duplicates)}. Image newNames must have unique names." | ||
| ) | ||
| sys.exit(1) | ||
|
|
||
| for image in images: | ||
| # Validate that the image has the required fields | ||
| if "name" not in image: | ||
| logger.fatal(f"Image {image} is missing the required 'name' field.") | ||
| return False | ||
| if "fromOverlay" in image: | ||
| if "newName" in image: | ||
| logger.fatal( | ||
| f"Image {image} cannot set newName when fromOverlay is set." | ||
| ) | ||
| return False | ||
| if "newTag" in image: | ||
| logger.fatal( | ||
| f"Image {image} cannot set newTag when fromOverlay is set." | ||
| ) | ||
| return False | ||
| else: | ||
| if ("newTag" not in image) and ("newName" not in image): | ||
| logger.fatal(f"Image {image} must set newName, newTag or both.") | ||
| return False | ||
| # Validate that the image has the required fields if it was not a dict, | ||
| # which means that it is coming from a promotion file and not from a | ||
| # kustomization.yaml file. | ||
| if not originally_dict and "overlays" not in image: | ||
| logger.fatal(f"Image {image} is missing the required 'overlays' field.") | ||
| return False | ||
|
|
||
| return True |
There was a problem hiding this comment.
✅ No longer an issue: Complex Method
validate_images is no longer above the threshold for cyclomatic complexity
Why does this problem occur?
This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring. Read more.
| def validate_charts(charts): | ||
| """ | ||
| Validate that the charts to update have the required fields. | ||
|
|
||
| Args: | ||
| charts (list): The list of charts to update. | ||
|
|
||
| Returns: | ||
| bool: True if all charts are valid, False otherwise. | ||
| """ | ||
| originally_dict = False | ||
| # Convert to list if it is a dict (which is the case when we are validating | ||
| # charts from a promotion file) | ||
| if isinstance(charts, dict): | ||
| originally_dict = True | ||
| charts = list(charts.values()) | ||
|
|
||
| # Ensure that all chart names are unique | ||
| duplicates = find_duplicates(charts, "name") | ||
| if len(duplicates) > 0: | ||
| logger.fatal( | ||
| f"Found duplicate chart names: {' '.join(duplicates)}. Charts must have unique names." | ||
| ) | ||
| sys.exit(1) | ||
|
|
||
| for chart in charts: | ||
| # Validate that the chart has the required fields | ||
| if "name" not in chart: | ||
| logger.fatal(f"Chart {chart} is missing the required 'name' field.") | ||
| return False | ||
| if "fromOverlay" in chart: | ||
| if "version" in chart: | ||
| logger.fatal( | ||
| f"Chart {chart} cannot set version when fromOverlay is set." | ||
| ) | ||
| return False | ||
| else: | ||
| if "version" not in chart: | ||
| logger.fatal(f"Chart {chart} must set version.") | ||
| return False | ||
| # Validate that the chart has the required fields if it was not a dict, | ||
| # which means that it is coming from a promotion file and not from a | ||
| # kustomization.yaml file. | ||
| if not originally_dict and "overlays" not in chart: | ||
| logger.fatal(f"Chart {chart} is missing the required 'overlays' field.") | ||
| return False | ||
|
|
||
| return True |
There was a problem hiding this comment.
✅ No longer an issue: Complex Method
validate_charts is no longer above the threshold for cyclomatic complexity
Why does this problem occur?
This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring. Read more.
| def validate_images(images): | ||
| """ | ||
| Validate a list of images to ensure they have the required fields and that the names and newNames are unique. | ||
|
|
||
| Args: | ||
| images (list): The list of images to validate. | ||
|
|
||
| Returns: | ||
| bool: True if all images are valid, False otherwise. | ||
| """ | ||
| originally_dict = False | ||
| # Convert to list if it is a dict (which is the case when we are validating | ||
| # images from a promotion file) | ||
| if isinstance(images, dict): | ||
| originally_dict = True | ||
| images = list(images.values()) | ||
|
|
||
| # Ensure that all image names are unique | ||
| duplicates = find_duplicates(images, "name") | ||
| if len(duplicates) > 0: | ||
| logger.fatal( | ||
| f"Found duplicate image names: {' '.join(duplicates)}. Images must have unique names." | ||
| ) | ||
| sys.exit(1) | ||
|
|
||
| # Ensure that all image newNames are unique | ||
| duplicates = find_duplicates(images, "newName") | ||
| if len(duplicates) > 0: | ||
| logger.fatal( | ||
| f"Found duplicate image newNames: {' '.join(duplicates)}. Image newNames must have unique names." | ||
| ) | ||
| sys.exit(1) | ||
|
|
||
| for image in images: | ||
| # Validate that the image has the required fields | ||
| if "name" not in image: | ||
| logger.fatal(f"Image {image} is missing the required 'name' field.") | ||
| return False | ||
| if "fromOverlay" in image: | ||
| if "newName" in image: | ||
| logger.fatal( | ||
| f"Image {image} cannot set newName when fromOverlay is set." | ||
| ) | ||
| return False | ||
| if "newTag" in image: | ||
| logger.fatal( | ||
| f"Image {image} cannot set newTag when fromOverlay is set." | ||
| ) | ||
| return False | ||
| else: | ||
| if ("newTag" not in image) and ("newName" not in image): | ||
| logger.fatal(f"Image {image} must set newName, newTag or both.") | ||
| return False | ||
| # Validate that the image has the required fields if it was not a dict, | ||
| # which means that it is coming from a promotion file and not from a | ||
| # kustomization.yaml file. | ||
| if not originally_dict and "overlays" not in image: | ||
| logger.fatal(f"Image {image} is missing the required 'overlays' field.") | ||
| return False | ||
|
|
||
| return True |
There was a problem hiding this comment.
✅ No longer an issue: Bumpy Road Ahead
validate_images is no longer above the threshold for logical blocks with deeply nested code
Why does this problem occur?
The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health. Read more.
| def validate_charts(charts): | ||
| """ | ||
| Validate that the charts to update have the required fields. | ||
|
|
||
| Args: | ||
| charts (list): The list of charts to update. | ||
|
|
||
| Returns: | ||
| bool: True if all charts are valid, False otherwise. | ||
| """ | ||
| originally_dict = False | ||
| # Convert to list if it is a dict (which is the case when we are validating | ||
| # charts from a promotion file) | ||
| if isinstance(charts, dict): | ||
| originally_dict = True | ||
| charts = list(charts.values()) | ||
|
|
||
| # Ensure that all chart names are unique | ||
| duplicates = find_duplicates(charts, "name") | ||
| if len(duplicates) > 0: | ||
| logger.fatal( | ||
| f"Found duplicate chart names: {' '.join(duplicates)}. Charts must have unique names." | ||
| ) | ||
| sys.exit(1) | ||
|
|
||
| for chart in charts: | ||
| # Validate that the chart has the required fields | ||
| if "name" not in chart: | ||
| logger.fatal(f"Chart {chart} is missing the required 'name' field.") | ||
| return False | ||
| if "fromOverlay" in chart: | ||
| if "version" in chart: | ||
| logger.fatal( | ||
| f"Chart {chart} cannot set version when fromOverlay is set." | ||
| ) | ||
| return False | ||
| else: | ||
| if "version" not in chart: | ||
| logger.fatal(f"Chart {chart} must set version.") | ||
| return False | ||
| # Validate that the chart has the required fields if it was not a dict, | ||
| # which means that it is coming from a promotion file and not from a | ||
| # kustomization.yaml file. | ||
| if not originally_dict and "overlays" not in chart: | ||
| logger.fatal(f"Chart {chart} is missing the required 'overlays' field.") | ||
| return False | ||
|
|
||
| return True |
There was a problem hiding this comment.
✅ No longer an issue: Bumpy Road Ahead
validate_charts is no longer above the threshold for logical blocks with deeply nested code
Why does this problem occur?
The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health. Read more.
There was a problem hiding this comment.
❌ Quality Gates: FAILED
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_images
- Complex Method validate.py: validate_charts
- Bumpy Road Ahead validate.py: validate_images
- Bumpy Road Ahead validate.py: validate_charts
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
There was a problem hiding this comment.
✅ Quality Gates: OK
- Declining Code Health: 0 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Inconclusive -- Not enough commits to recommend a review strategy. The recommendation will be enabled automatically once you have more development activity.
View detailed results in CodeScene
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
There was a problem hiding this comment.
❌ Quality Gates: FAILED
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_images
- Complex Method validate.py: validate_charts
- Bumpy Road Ahead validate.py: validate_images
- Bumpy Road Ahead validate.py: validate_charts
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
There was a problem hiding this comment.
❌ Quality Gates: FAILED
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_images
- Complex Method validate.py: validate_charts
- Bumpy Road Ahead validate.py: validate_images
- Bumpy Road Ahead validate.py: validate_charts
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
There was a problem hiding this comment.
❌ Quality Gates: FAILED
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_images
- Complex Method validate.py: validate_charts
- Bumpy Road Ahead validate.py: validate_images
- Bumpy Road Ahead validate.py: validate_charts
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
There was a problem hiding this comment.
❌ Quality Gates: FAILED
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_images
- Complex Method validate.py: validate_charts
- Bumpy Road Ahead validate.py: validate_images
- Bumpy Road Ahead validate.py: validate_charts
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
There was a problem hiding this comment.
✅ Quality Gates: OK
- Declining Code Health: 0 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Inconclusive -- Not enough commits to recommend a review strategy. The recommendation will be enabled automatically once you have more development activity.
View detailed results in CodeScene
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
There was a problem hiding this comment.
❌ Quality Gates: FAILED
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_images
- Complex Method validate.py: validate_charts
- Bumpy Road Ahead validate.py: validate_images
- Bumpy Road Ahead validate.py: validate_charts
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
There was a problem hiding this comment.
❌ Quality Gates: FAILED
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_images
- Complex Method validate.py: validate_charts
- Bumpy Road Ahead validate.py: validate_images
- Bumpy Road Ahead validate.py: validate_charts
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
There was a problem hiding this comment.
❌ Quality Gates: FAILED
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_images
- Complex Method validate.py: validate_charts
- Bumpy Road Ahead validate.py: validate_images
- Bumpy Road Ahead validate.py: validate_charts
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
| @@ -55,13 +55,43 @@ | |||
| import yaml | |||
|
|
|||
| from typing import Callable, Iterator, Union, Optional # noqa: F401 | |||
| from validate import validate_promotion_lists, validate_charts, validate_images | |||
There was a problem hiding this comment.
✅ Getting better: Overall Code Complexity
The mean cyclomatic complexity decreases from 6.71 to 6.31, threshold = 4
Why does this problem occur?
This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals. Read more.
There was a problem hiding this comment.
❌ Quality Gates: FAILED
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_images
- Complex Method validate.py: validate_charts
- Bumpy Road Ahead validate.py: validate_images
- Bumpy Road Ahead validate.py: validate_charts
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
There was a problem hiding this comment.
✅ Quality Gates: OK
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Inconclusive -- Not enough commits to recommend a review strategy. The recommendation will be enabled automatically once you have more development activity.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_image_fields
- Complex Method validate.py: validate_chart_fields
- Bumpy Road Ahead validate.py: validate_image_fields
- Bumpy Road Ahead validate.py: validate_chart_fields
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
| @@ -0,0 +1,231 @@ | |||
| import logging | |||
There was a problem hiding this comment.
❌ New issue: Complex Method
validate_image_fields has a cyclomatic complexity of 11, threshold = 9
Why does this problem occur?
This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring. Read more.
To ignore this warning click here.
There was a problem hiding this comment.
✅ Quality Gates: OK
- Declining Code Health: 1 findings(s) 🚩
- Improving Code Health: 5 findings(s) ✅
- Affected Hotspots: 1 files(s) 🔥
Recommended Review Level: Inconclusive -- Not enough commits to recommend a review strategy. The recommendation will be enabled automatically once you have more development activity.
View detailed results in CodeScene
🚩 Negative Code Health Impact (highest to lowest):
- Complex Method validate.py: validate_image_fields
✅ Positive Code Health Impact (highest to lowest):
- Overall Code Complexity promote.py 🔥
- Complex Method promote.py: validate_images 🔥
- Complex Method promote.py: validate_charts 🔥
- Bumpy Road Ahead promote.py: validate_images 🔥
- Bumpy Road Ahead promote.py: validate_charts 🔥
