-
Notifications
You must be signed in to change notification settings - Fork 376
.pr_agent_auto_best_practices
Pattern 1: Add defensive error handling for edge cases that could cause runtime errors, particularly for division by zero scenarios and parameter validation. Multiple suggestions show missing error handling causing potential crashes.
Example code before:
def calculate_percentage(covered, total):
return (covered / total) * 100
Example code after:
def calculate_percentage(covered, total):
return (covered / total) * 100 if total > 0 else 0.0
Relevant past accepted suggestions:
Suggestion 1:
Add protection against division by zero when calculating coverage percentages
Add error handling for division by zero when calculating total_coverage in CoverageReportFilter.filter_report()
cover_agent/coverage/processor.py [244-246]
-total_coverage=(sum(cov.covered_lines for cov in filtered_coverage.values()) /
- sum(cov.covered_lines + cov.missed_lines for cov in filtered_coverage.values()))
- if filtered_coverage else 0.0,
+total_lines = sum(len(cov.covered_lines) + len(cov.missed_lines) for cov in filtered_coverage.values())
+total_coverage = (sum(len(cov.covered_lines) for cov in filtered_coverage.values()) / total_lines) if total_lines > 0 else 0.0,Suggestion 2:
Add error handling for missing command line arguments in test command parsing
Add error handling for the case when 'pytest' is found in the command but the '--' substring is not found, which would cause an IndexError.
cover_agent/CoverAgent.py [72-75]
if 'pytest' in test_command:
ind1 = test_command.index('pytest')
- ind2 = test_command[ind1:].index('--')
- new_command_line = f"{test_command[:ind1]}pytest {test_file_relative_path} {test_command[ind1 + ind2:]}"
+ try:
+ ind2 = test_command[ind1:].index('--')
+ new_command_line = f"{test_command[:ind1]}pytest {test_file_relative_path} {test_command[ind1 + ind2:]}"
+ except ValueError:
+ new_command_line = f"{test_command[:ind1]}pytest {test_file_relative_path}"Suggestion 3:
Prevent potential division by zero error in token clipping function
In the clip_tokens function, handle the case where max_tokens is zero to prevent a potential division by zero error when calculating chars_per_token.
cover_agent/settings/token_handling.py [26-43]
def clip_tokens(text: str, max_tokens: int, add_three_dots=True, num_input_tokens=None, delete_last_line=False) -> str:
if not text:
return text
try:
if num_input_tokens is None:
encoder = TokenEncoder.get_token_encoder()
num_input_tokens = len(encoder.encode(text))
if num_input_tokens <= max_tokens:
return text
- if max_tokens < 0:
+ if max_tokens <= 0:
return ""
# calculate the number of characters to keep
num_chars = len(text)
chars_per_token = num_chars / num_input_tokens
factor = 0.9 # reduce by 10% to be safe
num_output_chars = int(factor * chars_per_token * max_tokens)Pattern 2: Validate input parameters and paths early to fail fast with clear error messages. Multiple suggestions show missing validation for critical inputs like file paths and folder locations.
Example code before:
def process_file(file_path):
with open(file_path) as f:
content = f.read()
Example code after:
def process_file(file_path):
if not os.path.exists(file_path):
raise ValueError(f"File not found: {file_path}")
with open(file_path) as f:
content = f.read()
Relevant past accepted suggestions:
Suggestion 1:
Fix incorrect parameter name and provide all required arguments for dataclass instantiation
Fix the typo in the parameter name 'coverage_percentag' to 'coverage_percentage' when creating CoverageData in JacocoProcessor.parse_coverage_report()
cover_agent/coverage/processor.py [167]
-coverage[class_name] = CoverageData(covered=covered, missed=missed, coverage_percentag=coverage_percentage)
+coverage[class_name] = CoverageData(covered_lines=[], covered=covered, missed_lines=[], missed=missed, coverage=coverage_percentage)Suggestion 2:
Validate test folder path exists before attempting to use it
Add validation to ensure that test_folder exists when provided, similar to the validation done for test_file. This prevents silent failures when an invalid folder is specified.
cover_agent/utils.py [303-305]
if hasattr(args, "test_folder") and args.test_folder:
+ test_folder_path = os.path.join(project_dir, args.test_folder)
+ if not os.path.exists(test_folder_path):
+ print(f"Test folder not found: `{test_folder_path}`, exiting.\n")
+ exit(-1)
if args.test_folder not in root:
continueSuggestion 3:
Handle empty project root in relative path calculation
Ensure that the get_included_files method handles the case where project_root is not provided or is an empty string. This could lead to unexpected behavior when calculating relative paths.
cover_agent/UnitTestGenerator.py [231-240]
def get_included_files(included_files: list, project_root: str = "", disable_tokens=False) -> str:
if included_files:
included_files_content = []
file_names_rel = []
for file_path in included_files:
try:
with open(file_path, "r") as file:
included_files_content.append(file.read())
- file_path_rel = os.path.relpath(file_path, project_root)
+ file_path_rel = os.path.relpath(file_path, project_root) if project_root else file_path
file_names_rel.append(file_path_rel)Pattern 3: Fix variable reference bugs where variables are used before definition or incorrect variable names are used. Multiple suggestions show issues with variable scoping and naming.
Example code before:
if value is None:
print("Value is missing")
value = compute_something()
Example code after:
value = compute_something()
if value is None:
print("Value is missing")
Relevant past accepted suggestions:
Suggestion 1:
Fix variable reference before assignment bug by removing unnecessary newline
The sourcefile variable is used before it's defined. Move the 'if sourcefile is None' check after the variable assignment to prevent NameError.
cover_agent/CoverageProcessor.py [241-244]
sourcefile = root.find(f".//sourcefile[@name='{class_name}.java']") or root.find(f".//sourcefile[@name='{class_name}.kt']")
-
if sourcefile is None:Suggestion 2:
Fix incorrect variable reference in logging statement to prevent undefined variable error
Fix incorrect variable reference in logging statement where coverage_percentages is used instead of new_coverage_percentages.
cover_agent/UnitTestValidator.py [565-567]
self.logger.info(
- f"Coverage for provided source file: {key} increased from {round(self.last_coverage_percentages[key] * 100, 2)} to {round(coverage_percentages[key] * 100, 2)}"
+ f"Coverage for provided source file: {key} increased from {round(self.last_coverage_percentages[key] * 100, 2)} to {round(new_coverage_percentages[key] * 100, 2)}"
)Pattern 4: Handle race conditions in file operations by capturing file metadata before processing. This pattern appears in suggestions dealing with file timestamps and modifications.
Example code before:
def process_file(file_path):
content = read_file(file_path)
timestamp = get_file_timestamp(file_path)
Example code after:
def process_file(file_path):
timestamp = get_file_timestamp(file_path)
content = read_file(file_path)
Relevant past accepted suggestions:
Suggestion 1:
Prevent race condition by capturing file modification time before processing
Fix potential race condition in process_coverage() by capturing file modification time before processing
cover_agent/coverage/processor.py [293]
-report = processor.process_coverage_report(time_of_test_command=int(os.path.getmtime(report_path) * 1000))
+report_mtime = int(os.path.getmtime(report_path) * 1000)
+report = processor.process_coverage_report(time_of_test_command=report_mtime)Pattern 5: Improve documentation clarity by adding explicit prerequisites and usage examples. Multiple suggestions focus on making the documentation more user-friendly.
Example code before:
## Usage
Run the command:
tool --input ... --output ...
Example code after:
## Prerequisites
- Python 3.8+
- Package X version Y
## Usage
Replace the placeholders with your values:
tool --input <input_file> --output <output_dir>
Relevant past accepted suggestions:
Suggestion 1:
Provide more context and explanation for the command usage in the README
Consider adding a brief explanation of what the cover-agent command does and what the ellipsis (...) represent in the command example. This will help users understand how to properly use the command.
3. Run the app
```shell
poetry run cover-agent \
- --source-file-path ... \
- ...
+ --source-file-path \
+ [other_options...]
```
+Replace `` with the actual path to your source file.
+Add any other necessary options as described in the [Running the Code](#running-the-code) section.
+
(i.e. prepending `poetry run` to your `cover-agent` commands --
-see the [Running the Code](#running-the-code) section above).
+see the [Running the Code](#running-the-code) section above for more details on available options).Suggestion 2:
Add prerequisites section to clarify system requirements before running the app locally
Consider adding a note about potential system requirements or dependencies that might be needed before running the app locally. This could include Python version, operating system compatibility, or any other prerequisites.
### Running the app locally from source
+
+#### Prerequisites
+- Python 3.x (specify minimum version)
+- Poetry
+
+#### Steps
1. Install the dependencies
```shell
poetry install
```
2. Let Poetry manage / create the environment
```shell
poetry shell
```
3. Run the app
```shell
poetry run cover-agent \
--source-file-path ... \
...
```[Auto-generated best practices - 2025-01-22]