You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Failed test name: TestConvertToMarkdown::test_simple_dictionary_input
Failure summary:
The action failed due to a test failure in the file 'test_convert_to_markdown.py'. Specifically:
The test 'test_simple_dictionary_input' in the 'TestConvertToMarkdown' class failed.
The failure occurred because the actual output of the 'convert_to_markdown_v2' function did not match the expected output.
The difference appears to be additional content in the actual output, including a section starting with "Here are some key observations to aid the review process:".
The assertion error suggests that while the beginning and end of the strings match, there is unexpected content in the middle.
Relevant error logs:
1: ##[group]Operating System2: Ubuntu
...
1006: tests/unittest/test_find_line_number_of_relevant_line_in_file.py ...... [ 62%]1007: tests/unittest/test_fix_output.py .... [ 67%]1008: tests/unittest/test_github_action_output.py .... [ 72%]1009: tests/unittest/test_handle_patch_deletions.py .... [ 76%]1010: tests/unittest/test_language_handler.py ...... [ 83%]1011: tests/unittest/test_load_yaml.py ... [ 87%]1012: tests/unittest/test_parse_code_suggestion.py .... [ 91%]1013: tests/unittest/test_try_fix_yaml.py ....... [100%]1014: =================================== FAILURES ===================================
...
1018: input_data = {'review': {1019: 'estimated_effort_to_review_[1-5]': '1, because the changes are minimal and straightforward, focusing on a single functionality addition.\n',1020: 'relevant_tests': 'No\n', 'possible_issues': 'No\n', 'security_concerns': 'No\n'}, 'code_feedback': [1021: {'relevant_file': '``pr_agent/git_providers/git_provider.py\n``', 'language': 'python\n',1022: 'suggestion': "Consider raising an exception or logging a warning when 'pr_url' attribute is not found. This can help in debugging issues related to the absence of 'pr_url' in instances where it's expected. [important]\n",1023: 'relevant_line': '[return ""](https://github.com/Codium-ai/pr-agent-pro/pull/102/files#diff-52d45f12b836f77ed1aef86e972e65404634ea4e2a6083fb71a9b0f9bb9e062fR199)'}]}
1024: expected_output = f'{PRReviewHeader.REGULAR.value} 🔍\n\n<table>\n<tr><td>⏱️ <strong>Estimated effort to review</strong>: 1 🔵⚪⚪⚪⚪</td></tr>\n<tr><td>🧪 <strong>No relevant tests</strong></td></tr>\n<tr><td>⚡ <strong>Possible issues</strong>: No\n</td></tr>\n<tr><td>🔒 <strong>No security concerns identified</strong></td></tr>\n</table>\n\n\n<details><summary> <strong>Code feedback:</strong></summary>\n\n<hr><table><tr><td>relevant file</td><td>pr_agent/git_providers/git_provider.py\n</td></tr><tr><td>suggestion </td><td>\n\n<strong>\n\nConsider raising an exception or logging a warning when \'pr_url\' attribute is not found. This can help in debugging issues related to the absence of \'pr_url\' in instances where it\'s expected. [important]\n\n</strong>\n</td></tr><tr><td>relevant line</td><td><a href=\'https://github.com/Codium-ai/pr-agent-pro/pull/102/files#diff-52d45f12b836f77ed1aef86e972e65404634ea4e2a6083fb71a9b0f9bb9e062fR199\'>return ""</a></td></tr></table><hr>\n\n</details>'
1025: > assert convert_to_markdown_v2(input_data).strip() == expected_output.strip()1026: E AssertionError: assert '## PR Review...n\n</details>' == '## PR Review...n\n</details>'
...
1028: E + 1029: E + Here are some key observations to aid the review process:1030: E 1031: E <table>1032: E <tr><td>⏱️ <strong>Estimated effort to review</strong>: 1 🔵⚪⚪⚪⚪</td></tr>1033: E <tr><td>🧪 <strong>No relevant tests</strong></td></tr>...1034: E 1035: E ...Full output truncated (19 lines hidden), use '-vv' to show1036: tests/unittest/test_convert_to_markdown.py:58: AssertionError
...
1060: /usr/local/lib/python3.12/site-packages/azure/devops/__init__.py:6: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('azure.devops')`.1061: Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages1062: pkg_resources.declare_namespace(__name__)1063: ../usr/local/lib/python3.12/site-packages/pkg_resources/__init__.py:23171064: /usr/local/lib/python3.12/site-packages/pkg_resources/__init__.py:2317: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('azure')`.1065: Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages1066: declare_namespace(parent)1067: ../usr/local/lib/python3.12/site-packages/pydantic/_internal/_config.py:2911068: /usr/local/lib/python3.12/site-packages/pydantic/_internal/_config.py:291: PydanticDeprecatedSince20: Support for class-based `config` is deprecated, use ConfigDict instead. Deprecated in Pydantic V2.0 to be removed in V3.0. See Pydantic V2 Migration Guide at https://errors.pydantic.dev/2.8/migration/
...
1160: pr_agent/tools/pr_questions.py 82 82 0%1161: pr_agent/tools/pr_reviewer.py 260 260 0%1162: pr_agent/tools/pr_similar_issue.py 363 363 0%1163: pr_agent/tools/pr_update_changelog.py 100 100 0%1164: ---------------------------------------------------------------------------------------1165: TOTAL 8593 7066 18%1166: Coverage XML written to file coverage.xml1167: =========================== short test summary info ============================1168: FAILED tests/unittest/test_convert_to_markdown.py::TestConvertToMarkdown::test_simple_dictionary_input1169: ================== 1 failed, 85 passed, 27 warnings in 10.17s ==================1170: ##[error]Process completed with exit code 1.
✨ CI feedback usage guide:
The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
The tool analyzes the failed checks and provides several feedbacks:
Failed stage
Failed test name
Failure summary
Relevant error logs
In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:
where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.
Configuration options
enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.
See more information about the checks tool in the docs.
Parameterize the test case to cover multiple scenarios
Consider parameterizing the test case to cover both scenarios where enable_intro_text is True and False. This will ensure that the new functionality is properly tested for both cases.
-expected_output = f'{PRReviewHeader.REGULAR.value} 🔍\n\nHere are some key observations to aid the review process:\n\n<table>\n<tr><td>⏱️ <strong>Estimated effort to review</strong>: 1 🔵⚪⚪⚪⚪</td></tr>\n<tr><td>🧪 <strong>No relevant tests</strong></td></tr>\n<tr><td>⚡ <strong>Possible issues</strong>: No\n</td></tr>\n<tr><td>🔒 <strong>No security concerns identified</strong></td></tr>\n</table>\n\n\n<details><summary> <strong>Code feedback:</strong></summary>\n\n<hr><table><tr><td>relevant file</td><td>pr_agent/git_providers/git_provider.py\n</td></tr><tr><td>suggestion </td><td>\n\n<strong>\n\nConsider raising an exception or logging a warning when \'pr_url\' attribute is not found. This can help in debugging issues related to the absence of \'pr_url\' in instances where it\'s expected. [important]\n\n</strong>\n</td></tr><tr><td>relevant line</td><td><a href=\'https://github.com/Codium-ai/pr-agent-pro/pull/102/files#diff-52d45f12b836f77ed1aef86e972e65404634ea4e2a6083fb71a9b0f9bb9e062fR199\'>return ""</a></td></tr></table><hr>\n\n</details>'
+@pytest.mark.parametrize("enable_intro_text, expected_intro", [+ (True, "Here are some key observations to aid the review process:\n\n"),+ (False, "")+])+def test_simple_dictionary_input(self, enable_intro_text, expected_intro):+ with patch('pr_agent.algo.utils.get_settings') as mock_get_settings:+ mock_get_settings.return_value = {"pr_reviewer.enable_intro_text": enable_intro_text}+ input_data = {'review': {+ 'estimated_effort_to_review_[1-5]': '1, because the changes are minimal and straightforward, focusing on a single functionality addition.\n',+ 'relevant_tests': 'No\n', 'possible_issues': 'No\n', 'security_concerns': 'No\n'}, 'code_feedback': [+ {'relevant_file': '``pr_agent/git_providers/git_provider.py\n``', 'language': 'python\n',+ 'suggestion': "Consider raising an exception or logging a warning when 'pr_url' attribute is not found. This can help in debugging issues related to the absence of 'pr_url' in instances where it's expected. [important]\n",+ 'relevant_line': '[return ""](https://github.com/Codium-ai/pr-agent-pro/pull/102/files#diff-52d45f12b836f77ed1aef86e972e65404634ea4e2a6083fb71a9b0f9bb9e062fR199)'}]}- assert convert_to_markdown_v2(input_data).strip() == expected_output.strip()
+ expected_output = f'{PRReviewHeader.REGULAR.value} 🔍\n\n{expected_intro}<table>\n<tr><td>⏱️ <strong>Estimated effort to review</strong>: 1 🔵⚪⚪⚪⚪</td></tr>\n<tr><td>🧪 <strong>No relevant tests</strong></td></tr>\n<tr><td>⚡ <strong>Possible issues</strong>: No\n</td></tr>\n<tr><td>🔒 <strong>No security concerns identified</strong></td></tr>\n</table>\n\n\n<details><summary> <strong>Code feedback:</strong></summary>\n\n<hr><table><tr><td>relevant file</td><td>pr_agent/git_providers/git_provider.py\n</td></tr><tr><td>suggestion </td><td>\n\n<strong>\n\nConsider raising an exception or logging a warning when \'pr_url\' attribute is not found. This can help in debugging issues related to the absence of \'pr_url\' in instances where it\'s expected. [important]\n\n</strong>\n</td></tr><tr><td>relevant line</td><td><a href=\'https://github.com/Codium-ai/pr-agent-pro/pull/102/files#diff-52d45f12b836f77ed1aef86e972e65404634ea4e2a6083fb71a9b0f9bb9e062fR199\'>return ""</a></td></tr></table><hr>\n\n</details>'
+ assert convert_to_markdown_v2(input_data).strip() == expected_output.strip()+
Apply this suggestion
Suggestion importance[1-10]: 8
Why: This suggestion significantly improves test coverage by ensuring both True and False scenarios for 'enable_intro_text' are tested. It's a valuable enhancement to the test suite.
8
Organization best practice
Use dictionary get() method with a default value for safer key access
Use the get() method with a default value when accessing the "pr_reviewer.enable_intro_text" key in the settings dictionary. This ensures that a default value (False in this case) is used if the key is not present, preventing potential KeyError exceptions.
Why: This suggestion improves code robustness by preventing potential KeyError exceptions. It's a good practice for dictionary access, especially when dealing with configuration settings.
7
Add a comment to explain the purpose of a new configuration option
Add a comment explaining the purpose and usage of the new enable_intro_text configuration option. This will improve code maintainability and help users understand how this option affects the PR review process.
+# Determines whether to include an introductory text in the PR review
enable_intro_text=true
Apply this suggestion
Suggestion importance[1-10]: 6
Why: Adding a comment to explain the new configuration option enhances code maintainability and user understanding. It's a good practice, especially for configuration files.
Improve configuration file organization and documentation
In configuration files, it's often beneficial to group related settings together and add comments explaining their purpose. Consider moving the enable_intro_text setting next to other related PR reviewer settings and adding a brief comment explaining its purpose.
-enable_intro_text=true+# Enable introductory text in PR reviews+enable_intro_text = true
Suggestion importance[1-10]: 8
Why: This suggestion significantly improves the configuration file's readability and maintainability. Adding comments and organizing related settings together is crucial for long-term project maintenance.
8
Maintainability
Use a constant for the introductory text to improve maintainability
Consider using a constant for the introductory text string. This would improve maintainability by centralizing the text in one place, making it easier to update or translate in the future. You could define a constant like INTRO_TEXT at the module level and use it in the f-string.
-markdown_text += f"Here are some relevant observations to aid the review process:\n\n"+INTRO_TEXT = "Here are some relevant observations to aid the review process:\n\n"+# ... (earlier in the file)+markdown_text += INTRO_TEXT
Suggestion importance[1-10]: 7
Why: This suggestion enhances code maintainability and makes future updates easier. It's a good practice for managing string constants, especially for potentially reusable text.
7
Organization best practice
Simplify the conditional check by removing the explicit False comparison
Instead of using get_settings().get("pr_reviewer.enable_intro_text", False), it's recommended to use the "implicit" false if possible. This aligns with Python's idiomatic way of checking for truthiness. A better approach would be if get_settings().get("pr_reviewer.enable_intro_text"):. This change simplifies the code and follows the Python convention of treating None as False in boolean contexts.
-if get_settings().get("pr_reviewer.enable_intro_text", False):+if get_settings().get("pr_reviewer.enable_intro_text"):
markdown_text += f"Here are some relevant observations to aid the review process:\n\n"
Suggestion importance[1-10]: 5
Why: The suggestion improves code readability and follows Python conventions. However, it's a minor optimization with minimal impact on functionality.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Type
Enhancement
Description
pr_agent/algo/utils.pyto add the intro text when enabledenable_intro_textinconfiguration.toml, set to true by defaultChanges walkthrough 📝
utils.py
Add optional introductory text for PR reviewspr_agent/algo/utils.py
reviews
configuration.toml
Add configuration for introductory text in PR reviewspr_agent/settings/configuration.toml