From 7074e45ec34aee588f9daf8f6cb960a1d5e7c9ef Mon Sep 17 00:00:00 2001 From: OpenHands Date: Fri, 15 Nov 2024 19:41:48 -0500 Subject: [PATCH 1/5] Fix issue #5059: [Bug]: Github resolver looking for wrong PR number (#5062) Co-authored-by: Graham Neubig --- openhands/resolver/issue_definitions.py | 41 +++++--- .../test_issue_handler_error_handling.py | 94 +++++++++++++++++++ tests/unit/resolver/test_issue_references.py | 34 +++++++ 3 files changed, 157 insertions(+), 12 deletions(-) create mode 100644 tests/unit/resolver/test_issue_handler_error_handling.py create mode 100644 tests/unit/resolver/test_issue_references.py diff --git a/openhands/resolver/issue_definitions.py b/openhands/resolver/issue_definitions.py index 2d40923ca568..10a134b812cf 100644 --- a/openhands/resolver/issue_definitions.py +++ b/openhands/resolver/issue_definitions.py @@ -83,7 +83,21 @@ def _extract_image_urls(self, issue_body: str) -> list[str]: return re.findall(image_pattern, issue_body) def _extract_issue_references(self, body: str) -> list[int]: - pattern = r'#(\d+)' + # First, remove code blocks as they may contain false positives + body = re.sub(r'```.*?```', '', body, flags=re.DOTALL) + + # Remove inline code + body = re.sub(r'`[^`]*`', '', body) + + # Remove URLs that contain hash symbols + body = re.sub(r'https?://[^\s)]*#\d+[^\s)]*', '', body) + + # Now extract issue numbers, making sure they're not part of other text + # The pattern matches #number that: + # 1. Is at the start of text or after whitespace/punctuation + # 2. Is followed by whitespace, punctuation, or end of text + # 3. Is not part of a URL + pattern = r'(?:^|[\s\[({]|[^\w#])#(\d+)(?=[\s,.\])}]|$)' return [int(match) for match in re.findall(pattern, body)] def _get_issue_comments( @@ -455,17 +469,20 @@ def __get_context_from_external_issues_references( ) for issue_number in unique_issue_references: - url = f'https://api.github.com/repos/{self.owner}/{self.repo}/issues/{issue_number}' - headers = { - 'Authorization': f'Bearer {self.token}', - 'Accept': 'application/vnd.github.v3+json', - } - response = requests.get(url, headers=headers) - response.raise_for_status() - issue_data = response.json() - issue_body = issue_data.get('body', '') - if issue_body: - closing_issues.append(issue_body) + try: + url = f'https://api.github.com/repos/{self.owner}/{self.repo}/issues/{issue_number}' + headers = { + 'Authorization': f'Bearer {self.token}', + 'Accept': 'application/vnd.github.v3+json', + } + response = requests.get(url, headers=headers) + response.raise_for_status() + issue_data = response.json() + issue_body = issue_data.get('body', '') + if issue_body: + closing_issues.append(issue_body) + except requests.exceptions.RequestException as e: + logger.warning(f'Failed to fetch issue {issue_number}: {str(e)}') return closing_issues diff --git a/tests/unit/resolver/test_issue_handler_error_handling.py b/tests/unit/resolver/test_issue_handler_error_handling.py new file mode 100644 index 000000000000..54adff3466fd --- /dev/null +++ b/tests/unit/resolver/test_issue_handler_error_handling.py @@ -0,0 +1,94 @@ +import pytest +import requests +from unittest.mock import patch, MagicMock + +from openhands.resolver.issue_definitions import PRHandler +from openhands.resolver.github_issue import ReviewThread + + +def test_handle_nonexistent_issue_reference(): + handler = PRHandler("test-owner", "test-repo", "test-token") + + # Mock the requests.get to simulate a 404 error + mock_response = MagicMock() + mock_response.raise_for_status.side_effect = requests.exceptions.HTTPError("404 Client Error: Not Found") + + with patch('requests.get', return_value=mock_response): + # Call the method with a non-existent issue reference + result = handler._PRHandler__get_context_from_external_issues_references( + closing_issues=[], + closing_issue_numbers=[], + issue_body="This references #999999", # Non-existent issue + review_comments=[], + review_threads=[], + thread_comments=None + ) + + # The method should return an empty list since the referenced issue couldn't be fetched + assert result == [] + + +def test_handle_rate_limit_error(): + handler = PRHandler("test-owner", "test-repo", "test-token") + + # Mock the requests.get to simulate a rate limit error + mock_response = MagicMock() + mock_response.raise_for_status.side_effect = requests.exceptions.HTTPError( + "403 Client Error: Rate Limit Exceeded" + ) + + with patch('requests.get', return_value=mock_response): + # Call the method with an issue reference + result = handler._PRHandler__get_context_from_external_issues_references( + closing_issues=[], + closing_issue_numbers=[], + issue_body="This references #123", + review_comments=[], + review_threads=[], + thread_comments=None + ) + + # The method should return an empty list since the request was rate limited + assert result == [] + + +def test_handle_network_error(): + handler = PRHandler("test-owner", "test-repo", "test-token") + + # Mock the requests.get to simulate a network error + with patch('requests.get', side_effect=requests.exceptions.ConnectionError("Network Error")): + # Call the method with an issue reference + result = handler._PRHandler__get_context_from_external_issues_references( + closing_issues=[], + closing_issue_numbers=[], + issue_body="This references #123", + review_comments=[], + review_threads=[], + thread_comments=None + ) + + # The method should return an empty list since the network request failed + assert result == [] + + +def test_successful_issue_reference(): + handler = PRHandler("test-owner", "test-repo", "test-token") + + # Mock a successful response + mock_response = MagicMock() + mock_response.raise_for_status.return_value = None + mock_response.json.return_value = {"body": "This is the referenced issue body"} + + with patch('requests.get', return_value=mock_response): + # Call the method with an issue reference + result = handler._PRHandler__get_context_from_external_issues_references( + closing_issues=[], + closing_issue_numbers=[], + issue_body="This references #123", + review_comments=[], + review_threads=[], + thread_comments=None + ) + + # The method should return a list with the referenced issue body + assert result == ["This is the referenced issue body"] \ No newline at end of file diff --git a/tests/unit/resolver/test_issue_references.py b/tests/unit/resolver/test_issue_references.py new file mode 100644 index 000000000000..e4da644983db --- /dev/null +++ b/tests/unit/resolver/test_issue_references.py @@ -0,0 +1,34 @@ +from openhands.resolver.issue_definitions import IssueHandler + + +def test_extract_issue_references(): + handler = IssueHandler("test-owner", "test-repo", "test-token") + + # Test basic issue reference + assert handler._extract_issue_references("Fixes #123") == [123] + + # Test multiple issue references + assert handler._extract_issue_references("Fixes #123, #456") == [123, 456] + + # Test issue references in code blocks should be ignored + assert handler._extract_issue_references(""" + Here's a code block: + ```python + # This is a comment with #123 + def func(): + pass # Another #456 + ``` + But this #789 should be extracted + """) == [789] + + # Test issue references in inline code should be ignored + assert handler._extract_issue_references("This `#123` should be ignored but #456 should be extracted") == [456] + + # Test issue references in URLs should be ignored + assert handler._extract_issue_references("Check http://example.com/#123 but #456 should be extracted") == [456] + + # Test issue references in markdown links should be extracted + assert handler._extract_issue_references("[Link to #123](http://example.com) and #456") == [123, 456] + + # Test issue references with text around them + assert handler._extract_issue_references("Issue #123 is fixed and #456 is pending") == [123, 456] From 2b7932b46c37aec05e867e5a4b7f724e41c6f89a Mon Sep 17 00:00:00 2001 From: OpenHands Date: Fri, 15 Nov 2024 20:43:49 -0500 Subject: [PATCH 2/5] Fix issue #5070: [Bug]: lint-fix workflow is failing (#5078) --- .github/workflows/lint-fix.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint-fix.yml b/.github/workflows/lint-fix.yml index a882d6802946..78fdef9afcfb 100644 --- a/.github/workflows/lint-fix.yml +++ b/.github/workflows/lint-fix.yml @@ -43,7 +43,7 @@ jobs: run: pip install pre-commit==3.7.0 - name: Fix python lint issues run: | - pre-commit run --files openhands/**/* evaluation/**/* tests/**/* --config ./dev_config/python/.pre-commit-config.yaml --all-files + pre-commit run --files openhands/**/* evaluation/**/* tests/**/* --config ./dev_config/python/.pre-commit-config.yaml # Commit and push changes if any - name: Check for changes From f7652bd55889de52dcfef07c8a03cb4515b3bebb Mon Sep 17 00:00:00 2001 From: OpenHands Date: Sat, 16 Nov 2024 00:55:41 -0500 Subject: [PATCH 3/5] Fix issue #5080: [Bug]: lint-fix.yml github action doesn't work on a branch not from this repo (#5081) --- .github/workflows/lint-fix.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/lint-fix.yml b/.github/workflows/lint-fix.yml index 78fdef9afcfb..e3021b887c53 100644 --- a/.github/workflows/lint-fix.yml +++ b/.github/workflows/lint-fix.yml @@ -16,6 +16,7 @@ jobs: - uses: actions/checkout@v4 with: ref: ${{ github.head_ref }} + repository: ${{ github.event.pull_request.head.repo.full_name }} fetch-depth: 0 token: ${{ secrets.GITHUB_TOKEN }} From 9d47ddba38d27b1222df795cfd96285c69609f45 Mon Sep 17 00:00:00 2001 From: "sp.wack" <83104063+amanape@users.noreply.github.com> Date: Sat, 16 Nov 2024 07:57:41 +0200 Subject: [PATCH 4/5] Reduce output from frontend tests (#5023) --- frontend/vite.config.ts | 1 + frontend/vitest.setup.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index 4c403aca8496..6ec8467fc2c7 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -91,6 +91,7 @@ export default defineConfig(({ mode }) => { test: { environment: "jsdom", setupFiles: ["vitest.setup.ts"], + reporters: "basic", exclude: [...configDefaults.exclude, "tests"], coverage: { reporter: ["text", "json", "html", "lcov", "text-summary"], diff --git a/frontend/vitest.setup.ts b/frontend/vitest.setup.ts index 9ef8e081682e..5ee3071b3ea6 100644 --- a/frontend/vitest.setup.ts +++ b/frontend/vitest.setup.ts @@ -13,7 +13,7 @@ vi.mock("react-i18next", async (importOriginal) => ({ })); // Mock requests during tests -beforeAll(() => server.listen()); +beforeAll(() => server.listen({ onUnhandledRequest: "bypass" })); afterEach(() => { server.resetHandlers(); // Cleanup the document body after each test From 97f3249205e25abafc13e2c42037dd9b309fbe73 Mon Sep 17 00:00:00 2001 From: "Ryan H. Tran" Date: Sat, 16 Nov 2024 12:58:26 +0700 Subject: [PATCH 5/5] Move linter and diff utils to openhands-aci (#5020) --- openhands/linter/__init__.py | 8 +- openhands/linter/base.py | 79 ---- openhands/linter/languages/python.py | 98 ----- openhands/linter/languages/treesitter.py | 74 ---- openhands/linter/linter.py | 122 ------ openhands/linter/utils/__init__.py | 3 - openhands/linter/utils/cmd.py | 37 -- openhands/runtime/utils/edit.py | 9 +- openhands/utils/diff.py | 41 -- poetry.lock | 13 +- pyproject.toml | 2 +- tests/runtime/test_edit.py | 8 +- tests/unit/linters/conftest.py | 75 ---- tests/unit/linters/test_lint_diff.py | 417 ------------------- tests/unit/linters/test_python_linter.py | 84 ---- tests/unit/linters/test_treesitter_linter.py | 113 ----- tests/unit/linters/test_visualize.py | 86 ---- 17 files changed, 18 insertions(+), 1251 deletions(-) delete mode 100644 openhands/linter/base.py delete mode 100644 openhands/linter/languages/python.py delete mode 100644 openhands/linter/languages/treesitter.py delete mode 100644 openhands/linter/linter.py delete mode 100644 openhands/linter/utils/__init__.py delete mode 100644 openhands/linter/utils/cmd.py delete mode 100644 openhands/utils/diff.py delete mode 100644 tests/unit/linters/conftest.py delete mode 100644 tests/unit/linters/test_lint_diff.py delete mode 100644 tests/unit/linters/test_python_linter.py delete mode 100644 tests/unit/linters/test_treesitter_linter.py delete mode 100644 tests/unit/linters/test_visualize.py diff --git a/openhands/linter/__init__.py b/openhands/linter/__init__.py index 43d3b07d7d01..23e5d0de6e48 100644 --- a/openhands/linter/__init__.py +++ b/openhands/linter/__init__.py @@ -1,9 +1,11 @@ """Linter module for OpenHands. -Part of this Linter module is adapted from Aider (Apache 2.0 License, [original code](https://github.com/paul-gauthier/aider/blob/main/aider/linter.py)). Please see the [original repository](https://github.com/paul-gauthier/aider) for more information. +Part of this Linter module is adapted from Aider (Apache 2.0 License, [original +code](https://github.com/paul-gauthier/aider/blob/main/aider/linter.py)). +- Please see the [original repository](https://github.com/paul-gauthier/aider) for more information. +- The detailed implementation of the linter can be found at: https://github.com/All-Hands-AI/openhands-aci. """ -from openhands.linter.base import LintResult -from openhands.linter.linter import DefaultLinter +from openhands_aci.linter import DefaultLinter, LintResult __all__ = ['DefaultLinter', 'LintResult'] diff --git a/openhands/linter/base.py b/openhands/linter/base.py deleted file mode 100644 index e50b750b4880..000000000000 --- a/openhands/linter/base.py +++ /dev/null @@ -1,79 +0,0 @@ -from abc import ABC, abstractmethod - -from pydantic import BaseModel - - -class LintResult(BaseModel): - file: str - line: int # 1-indexed - column: int # 1-indexed - message: str - - def visualize(self, half_window: int = 3) -> str: - """Visualize the lint result by print out all the lines where the lint result is found. - - Args: - half_window: The number of context lines to display around the error on each side. - """ - with open(self.file, 'r') as f: - file_lines = f.readlines() - - # Add line numbers - _span_size = len(str(len(file_lines))) - file_lines = [ - f'{i + 1:>{_span_size}}|{line.rstrip()}' - for i, line in enumerate(file_lines) - ] - - # Get the window of lines to display - assert self.line <= len(file_lines) and self.line > 0 - line_idx = self.line - 1 - begin_window = max(0, line_idx - half_window) - end_window = min(len(file_lines), line_idx + half_window + 1) - - selected_lines = file_lines[begin_window:end_window] - line_idx_in_window = line_idx - begin_window - - # Add character hint - _character_hint = ( - _span_size * ' ' - + ' ' * (self.column) - + '^' - + ' ERROR HERE: ' - + self.message - ) - selected_lines[line_idx_in_window] = ( - f'\033[91m{selected_lines[line_idx_in_window]}\033[0m' - + '\n' - + _character_hint - ) - return '\n'.join(selected_lines) - - -class LinterException(Exception): - """Base class for all linter exceptions.""" - - pass - - -class BaseLinter(ABC): - """Base class for all linters. - - Each linter should be able to lint files of a specific type and return a list of (parsed) lint results. - """ - - encoding: str = 'utf-8' - - @property - @abstractmethod - def supported_extensions(self) -> list[str]: - """The file extensions that this linter supports, such as .py or .tsx.""" - return [] - - @abstractmethod - def lint(self, file_path: str) -> list[LintResult]: - """Lint the given file. - - file_path: The path to the file to lint. Required to be absolute. - """ - pass diff --git a/openhands/linter/languages/python.py b/openhands/linter/languages/python.py deleted file mode 100644 index 9b7e944a2868..000000000000 --- a/openhands/linter/languages/python.py +++ /dev/null @@ -1,98 +0,0 @@ -from typing import List - -from openhands.core.logger import openhands_logger as logger -from openhands.linter.base import BaseLinter, LintResult -from openhands.linter.utils import run_cmd - - -def python_compile_lint(fname: str) -> list[LintResult]: - try: - with open(fname, 'r') as f: - code = f.read() - compile(code, fname, 'exec') # USE TRACEBACK BELOW HERE - return [] - except SyntaxError as err: - err_lineno = getattr(err, 'end_lineno', err.lineno) - err_offset = getattr(err, 'end_offset', err.offset) - if err_offset and err_offset < 0: - err_offset = err.offset - return [ - LintResult( - file=fname, line=err_lineno, column=err_offset or 1, message=err.msg - ) - ] - - -def flake_lint(filepath: str) -> list[LintResult]: - fatal = 'F821,F822,F831,E112,E113,E999,E902' - flake8_cmd = f'flake8 --select={fatal} --isolated {filepath}' - - try: - cmd_outputs = run_cmd(flake8_cmd) - except FileNotFoundError: - return [] - results: list[LintResult] = [] - if not cmd_outputs: - return results - for line in cmd_outputs.splitlines(): - parts = line.split(':') - if len(parts) >= 4: - _msg = parts[3].strip() - if len(parts) > 4: - _msg += ': ' + parts[4].strip() - - try: - line_num = int(parts[1]) - except ValueError as e: - logger.warning( - f'Error parsing flake8 output for line: {e}. Parsed parts: {parts}. Skipping...' - ) - continue - - try: - column_num = int(parts[2]) - except ValueError as e: - column_num = 1 - _msg = ( - parts[2].strip() + ' ' + _msg - ) # add the unparsed message to the original message - logger.warning( - f'Error parsing flake8 output for column: {e}. Parsed parts: {parts}. Using default column 1.' - ) - - results.append( - LintResult( - file=filepath, - line=line_num, - column=column_num, - message=_msg, - ) - ) - return results - - -class PythonLinter(BaseLinter): - @property - def supported_extensions(self) -> List[str]: - return ['.py'] - - def lint(self, file_path: str) -> list[LintResult]: - error = flake_lint(file_path) - if not error: - error = python_compile_lint(file_path) - return error - - def compile_lint(self, file_path: str, code: str) -> List[LintResult]: - try: - compile(code, file_path, 'exec') - return [] - except SyntaxError as e: - return [ - LintResult( - file=file_path, - line=e.lineno, - column=e.offset, - message=str(e), - rule='SyntaxError', - ) - ] diff --git a/openhands/linter/languages/treesitter.py b/openhands/linter/languages/treesitter.py deleted file mode 100644 index 83b5d466aecc..000000000000 --- a/openhands/linter/languages/treesitter.py +++ /dev/null @@ -1,74 +0,0 @@ -import warnings - -from grep_ast import TreeContext, filename_to_lang -from grep_ast.parsers import PARSERS -from tree_sitter_languages import get_parser - -from openhands.linter.base import BaseLinter, LintResult - -# tree_sitter is throwing a FutureWarning -warnings.simplefilter('ignore', category=FutureWarning) - - -def tree_context(fname, code, line_nums): - context = TreeContext( - fname, - code, - color=False, - line_number=True, - child_context=False, - last_line=False, - margin=0, - mark_lois=True, - loi_pad=3, - # header_max=30, - show_top_of_file_parent_scope=False, - ) - line_nums = set(line_nums) - context.add_lines_of_interest(line_nums) - context.add_context() - output = context.format() - return output - - -def traverse_tree(node): - """Traverses the tree to find errors.""" - errors = [] - if node.type == 'ERROR' or node.is_missing: - line_no = node.start_point[0] + 1 - col_no = node.start_point[1] + 1 - error_type = 'Missing node' if node.is_missing else 'Syntax error' - errors.append((line_no, col_no, error_type)) - - for child in node.children: - errors += traverse_tree(child) - - return errors - - -class TreesitterBasicLinter(BaseLinter): - @property - def supported_extensions(self) -> list[str]: - return list(PARSERS.keys()) - - def lint(self, file_path: str) -> list[LintResult]: - """Use tree-sitter to look for syntax errors, display them with tree context.""" - lang = filename_to_lang(file_path) - if not lang: - return [] - parser = get_parser(lang) - with open(file_path, 'r') as f: - code = f.read() - tree = parser.parse(bytes(code, 'utf-8')) - errors = traverse_tree(tree.root_node) - if not errors: - return [] - return [ - LintResult( - file=file_path, - line=int(line), - column=int(col), - message=error_details, - ) - for line, col, error_details in errors - ] diff --git a/openhands/linter/linter.py b/openhands/linter/linter.py deleted file mode 100644 index a7a4a23ca2c1..000000000000 --- a/openhands/linter/linter.py +++ /dev/null @@ -1,122 +0,0 @@ -import os -from collections import defaultdict -from difflib import SequenceMatcher - -from openhands.linter.base import BaseLinter, LinterException, LintResult -from openhands.linter.languages.python import PythonLinter -from openhands.linter.languages.treesitter import TreesitterBasicLinter - - -class DefaultLinter(BaseLinter): - def __init__(self): - self.linters: dict[str, list[BaseLinter]] = defaultdict(list) - self.linters['.py'] = [PythonLinter()] - - # Add treesitter linter as a fallback for all linters - self.basic_linter = TreesitterBasicLinter() - for extension in self.basic_linter.supported_extensions: - self.linters[extension].append(self.basic_linter) - self._supported_extensions = list(self.linters.keys()) - - @property - def supported_extensions(self) -> list[str]: - return self._supported_extensions - - def lint(self, file_path: str) -> list[LintResult]: - if not os.path.isabs(file_path): - raise LinterException(f'File path {file_path} is not an absolute path') - file_extension = os.path.splitext(file_path)[1] - - linters: list[BaseLinter] = self.linters.get(file_extension, []) - for linter in linters: - res = linter.lint(file_path) - # We always return the first linter's result (higher priority) - if res: - return res - return [] - - def lint_file_diff( - self, original_file_path: str, updated_file_path: str - ) -> list[LintResult]: - """Only return lint errors that are introduced by the diff. - - Args: - original_file_path: The original file path. - updated_file_path: The updated file path. - - Returns: - A list of lint errors that are introduced by the diff. - """ - # 1. Lint the original and updated file - original_lint_errors: list[LintResult] = self.lint(original_file_path) - updated_lint_errors: list[LintResult] = self.lint(updated_file_path) - - # 2. Load the original and updated file content - with open(original_file_path, 'r') as f: - old_lines = f.readlines() - with open(updated_file_path, 'r') as f: - new_lines = f.readlines() - - # 3. Get line numbers that are changed & unchanged - # Map the line number of the original file to the updated file - # NOTE: this only works for lines that are not changed (i.e., equal) - old_to_new_line_no_mapping: dict[int, int] = {} - replace_or_inserted_lines: list[int] = [] - for ( - tag, - old_idx_start, - old_idx_end, - new_idx_start, - new_idx_end, - ) in SequenceMatcher( - isjunk=None, - a=old_lines, - b=new_lines, - ).get_opcodes(): - if tag == 'equal': - for idx, _ in enumerate(old_lines[old_idx_start:old_idx_end]): - old_to_new_line_no_mapping[old_idx_start + idx + 1] = ( - new_idx_start + idx + 1 - ) - elif tag == 'replace' or tag == 'insert': - for idx, _ in enumerate(old_lines[old_idx_start:old_idx_end]): - replace_or_inserted_lines.append(new_idx_start + idx + 1) - else: - # omit the case of delete - pass - - # 4. Get pre-existing errors in unchanged lines - # increased error elsewhere introduced by the newlines - # i.e., we omit errors that are already in original files and report new one - new_line_no_to_original_errors: dict[int, list[LintResult]] = defaultdict(list) - for error in original_lint_errors: - if error.line in old_to_new_line_no_mapping: - new_line_no_to_original_errors[ - old_to_new_line_no_mapping[error.line] - ].append(error) - - # 5. Select errors from lint results in new file to report - selected_errors = [] - for error in updated_lint_errors: - # 5.1. Error introduced by replace/insert - if error.line in replace_or_inserted_lines: - selected_errors.append(error) - # 5.2. Error introduced by modified lines that impacted - # the unchanged lines that HAVE pre-existing errors - elif error.line in new_line_no_to_original_errors: - # skip if the error is already reported - # or add if the error is new - if not any( - original_error.message == error.message - and original_error.column == error.column - for original_error in new_line_no_to_original_errors[error.line] - ): - selected_errors.append(error) - # 5.3. Error introduced by modified lines that impacted - # the unchanged lines that have NO pre-existing errors - else: - selected_errors.append(error) - - # 6. Sort errors by line and column - selected_errors.sort(key=lambda x: (x.line, x.column)) - return selected_errors diff --git a/openhands/linter/utils/__init__.py b/openhands/linter/utils/__init__.py deleted file mode 100644 index e48f26f076b5..000000000000 --- a/openhands/linter/utils/__init__.py +++ /dev/null @@ -1,3 +0,0 @@ -from .cmd import check_tool_installed, run_cmd - -__all__ = ['run_cmd', 'check_tool_installed'] diff --git a/openhands/linter/utils/cmd.py b/openhands/linter/utils/cmd.py deleted file mode 100644 index f5c2803c3d77..000000000000 --- a/openhands/linter/utils/cmd.py +++ /dev/null @@ -1,37 +0,0 @@ -import os -import subprocess - - -def run_cmd(cmd: str, cwd: str | None = None) -> str | None: - """Run a command and return the output. - - If the command succeeds, return None. If the command fails, return the stdout. - """ - - process = subprocess.Popen( - cmd.split(), - cwd=cwd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - encoding='utf-8', - errors='replace', - ) - stdout, _ = process.communicate() - if process.returncode == 0: - return None - return stdout - - -def check_tool_installed(tool_name: str) -> bool: - """Check if a tool is installed.""" - try: - subprocess.run( - [tool_name, '--version'], - check=True, - cwd=os.getcwd(), - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) - return True - except (subprocess.CalledProcessError, FileNotFoundError): - return False diff --git a/openhands/runtime/utils/edit.py b/openhands/runtime/utils/edit.py index cd3ffd0b71ce..bcb876f865d9 100644 --- a/openhands/runtime/utils/edit.py +++ b/openhands/runtime/utils/edit.py @@ -4,13 +4,11 @@ import tempfile from abc import ABC, abstractmethod +from openhands_aci.utils.diff import get_diff + from openhands.core.config import AppConfig from openhands.core.logger import openhands_logger as logger -from openhands.events.action import ( - FileEditAction, - FileReadAction, - FileWriteAction, -) +from openhands.events.action import FileEditAction, FileReadAction, FileWriteAction from openhands.events.observation import ( ErrorObservation, FileEditObservation, @@ -22,7 +20,6 @@ from openhands.llm.llm import LLM from openhands.llm.metrics import Metrics from openhands.utils.chunk_localizer import Chunk, get_top_k_chunk_matches -from openhands.utils.diff import get_diff SYS_MSG = """Your job is to produce a new version of the file based on the old version and the provided draft of the new version. The provided draft may be incomplete (it may skip lines) and/or incorrectly indented. You should try to apply the changes present in the draft to the old version, and output a new version of the file. diff --git a/openhands/utils/diff.py b/openhands/utils/diff.py deleted file mode 100644 index 71d00a2eb943..000000000000 --- a/openhands/utils/diff.py +++ /dev/null @@ -1,41 +0,0 @@ -import difflib - -import whatthepatch - - -def get_diff(old_contents: str, new_contents: str, filepath: str = 'file') -> str: - diff = list( - difflib.unified_diff( - old_contents.split('\n'), - new_contents.split('\n'), - fromfile=filepath, - tofile=filepath, - # do not output unchange lines - # because they can cause `parse_diff` to fail - n=0, - ) - ) - return '\n'.join(map(lambda x: x.rstrip(), diff)) - - -def parse_diff(diff_patch: str) -> list[whatthepatch.patch.Change]: - # handle empty patch - if diff_patch.strip() == '': - return [] - - patch = whatthepatch.parse_patch(diff_patch) - patch_list = list(patch) - assert len(patch_list) == 1, ( - 'parse_diff only supports single file diff. But got:\nPATCH:\n' - + diff_patch - + '\nPATCH LIST:\n' - + str(patch_list) - ) - changes = patch_list[0].changes - - # ignore changes that are the same (i.e., old_lineno == new_lineno) - output_changes = [] - for change in changes: - if change.old != change.new: - output_changes.append(change) - return output_changes diff --git a/poetry.lock b/poetry.lock index 80d6b20882c8..70d888cafed6 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. [[package]] name = "aenum" @@ -5629,7 +5629,6 @@ optional = false python-versions = ">=3.6" files = [ {file = "opencv-python-4.10.0.84.tar.gz", hash = "sha256:72d234e4582e9658ffea8e9cae5b63d488ad06994ef12d81dc303b17472f3526"}, - {file = "opencv_python-4.10.0.84-cp37-abi3-macosx_11_0_arm64.whl", hash = "sha256:fc182f8f4cda51b45f01c64e4cbedfc2f00aff799debebc305d8d0210c43f251"}, {file = "opencv_python-4.10.0.84-cp37-abi3-macosx_12_0_x86_64.whl", hash = "sha256:71e575744f1d23f79741450254660442785f45a0797212852ee5199ef12eed98"}, {file = "opencv_python-4.10.0.84-cp37-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:09a332b50488e2dda866a6c5573ee192fe3583239fb26ff2f7f9ceb0bc119ea6"}, {file = "opencv_python-4.10.0.84-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:9ace140fc6d647fbe1c692bcb2abce768973491222c067c131d80957c595b71f"}, @@ -5642,17 +5641,18 @@ numpy = {version = ">=1.26.0", markers = "python_version >= \"3.12\""} [[package]] name = "openhands-aci" -version = "0.1.0" +version = "0.1.1" description = "An Agent-Computer Interface (ACI) designed for software development agents OpenHands." optional = false python-versions = "<4.0,>=3.12" files = [ - {file = "openhands_aci-0.1.0-py3-none-any.whl", hash = "sha256:f28e5a32e394d1e643f79bf8af27fe44d039cb71729d590f9f3ee0c23c075f00"}, - {file = "openhands_aci-0.1.0.tar.gz", hash = "sha256:babc55f516efbb27eb7e528662e14b75c902965c48a110408fda824b83ea4461"}, + {file = "openhands_aci-0.1.1-py3-none-any.whl", hash = "sha256:8831f97b887571005dca0d70a9f6f0a4f9feb35d3d41f499e70d72b5fb68a599"}, + {file = "openhands_aci-0.1.1.tar.gz", hash = "sha256:705b74a12a8f428e64295b5de125f553500f62ef5ab3a5a6284d8fcf638025e6"}, ] [package.dependencies] diskcache = ">=5.6.3,<6.0.0" +flake8 = "*" gitpython = "*" grep-ast = "0.3.3" litellm = "*" @@ -5661,6 +5661,7 @@ numpy = "*" pandas = "*" scipy = "*" tree-sitter = "0.21.3" +whatthepatch = ">=1.0.6,<2.0.0" [[package]] name = "opentelemetry-api" @@ -10211,4 +10212,4 @@ testing = ["coverage[toml]", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "^3.12" -content-hash = "8718ffe2ed836fca6c646c37bdad2c9c8e63ebd7ec881f420148fef5095d19e4" +content-hash = "b710448cff0788b563f4d7614fca438ab0b9fe19903a061750012c56da95ff37" diff --git a/pyproject.toml b/pyproject.toml index ce49694e14aa..1caad8bf9fc7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,7 +63,7 @@ opentelemetry-exporter-otlp-proto-grpc = "1.25.0" modal = "^0.64.145" runloop-api-client = "0.7.0" pygithub = "^2.5.0" -openhands-aci = "^0.1.0" +openhands-aci = "^0.1.1" [tool.poetry.group.llama-index.dependencies] llama-index = "*" diff --git a/tests/runtime/test_edit.py b/tests/runtime/test_edit.py index b1508af226c1..99a7ce113b18 100644 --- a/tests/runtime/test_edit.py +++ b/tests/runtime/test_edit.py @@ -3,16 +3,12 @@ import os import pytest -from conftest import ( - TEST_IN_CI, - _close_test_runtime, - _load_runtime, -) +from conftest import TEST_IN_CI, _close_test_runtime, _load_runtime +from openhands_aci.utils.diff import get_diff from openhands.core.logger import openhands_logger as logger from openhands.events.action import FileEditAction, FileReadAction from openhands.events.observation import FileEditObservation -from openhands.utils.diff import get_diff ORGINAL = """from flask import Flask app = Flask(__name__) diff --git a/tests/unit/linters/conftest.py b/tests/unit/linters/conftest.py deleted file mode 100644 index 4a2b51812bb9..000000000000 --- a/tests/unit/linters/conftest.py +++ /dev/null @@ -1,75 +0,0 @@ -import pytest - - -@pytest.fixture -def syntax_error_py_file(tmp_path): - file_content = """ - def foo(): - print("Hello, World!") - print("Wrong indent") - foo( - """ - file_path = tmp_path / 'test_file.py' - file_path.write_text(file_content) - return str(file_path) - - -@pytest.fixture -def wrongly_indented_py_file(tmp_path): - file_content = """ - def foo(): - print("Hello, World!") - """ - file_path = tmp_path / 'test_file.py' - file_path.write_text(file_content) - return str(file_path) - - -@pytest.fixture -def simple_correct_py_file(tmp_path): - file_content = 'print("Hello, World!")\n' - file_path = tmp_path / 'test_file.py' - file_path.write_text(file_content) - return str(file_path) - - -@pytest.fixture -def simple_correct_py_func_def(tmp_path): - file_content = """def foo(): - print("Hello, World!") -foo() -""" - file_path = tmp_path / 'test_file.py' - file_path.write_text(file_content) - return str(file_path) - - -@pytest.fixture -def simple_correct_ruby_file(tmp_path): - file_content = """def foo - puts "Hello, World!" -end -foo -""" - file_path = tmp_path / 'test_file.rb' - file_path.write_text(file_content) - return str(file_path) - - -@pytest.fixture -def simple_incorrect_ruby_file(tmp_path): - file_content = """def foo(): - print("Hello, World!") -foo() -""" - file_path = tmp_path / 'test_file.rb' - file_path.write_text(file_content) - return str(file_path) - - -@pytest.fixture -def parenthesis_incorrect_ruby_file(tmp_path): - file_content = """def print_hello_world()\n puts 'Hello World'\n""" - file_path = tmp_path / 'test_file.rb' - file_path.write_text(file_content) - return str(file_path) diff --git a/tests/unit/linters/test_lint_diff.py b/tests/unit/linters/test_lint_diff.py deleted file mode 100644 index f3b560c3df32..000000000000 --- a/tests/unit/linters/test_lint_diff.py +++ /dev/null @@ -1,417 +0,0 @@ -from openhands.linter import DefaultLinter, LintResult -from openhands.utils.diff import get_diff, parse_diff - -OLD_CONTENT = """ -def foo(): - print("Hello, World!") - x = UNDEFINED_VARIABLE -foo() -""" - -NEW_CONTENT_V1 = ( - OLD_CONTENT - + """ -def new_function_that_causes_error(): - y = ANOTHER_UNDEFINED_VARIABLE -""" -) - -NEW_CONTENT_V2 = """ -def foo(): - print("Hello, World!") - x = UNDEFINED_VARIABLE - y = ANOTHER_UNDEFINED_VARIABLE -foo() -""" - - -def test_get_and_parse_diff(tmp_path): - diff = get_diff(OLD_CONTENT, NEW_CONTENT_V1, 'test.py') - print(diff) - assert ( - diff - == """ ---- test.py -+++ test.py -@@ -6,0 +7,3 @@ -+def new_function_that_causes_error(): -+ y = ANOTHER_UNDEFINED_VARIABLE -+ -""".strip() - ) - - print( - '\n'.join( - [f'{i+1}|{line}' for i, line in enumerate(NEW_CONTENT_V1.splitlines())] - ) - ) - changes = parse_diff(diff) - assert len(changes) == 3 - assert ( - changes[0].old is None - and changes[0].new == 7 - and changes[0].line == 'def new_function_that_causes_error():' - ) - assert ( - changes[1].old is None - and changes[1].new == 8 - and changes[1].line == ' y = ANOTHER_UNDEFINED_VARIABLE' - ) - assert changes[2].old is None and changes[2].new == 9 and changes[2].line == '' - - -def test_lint_with_diff_append(tmp_path): - with open(tmp_path / 'old.py', 'w') as f: - f.write(OLD_CONTENT) - with open(tmp_path / 'new.py', 'w') as f: - f.write(NEW_CONTENT_V1) - - linter = DefaultLinter() - result: list[LintResult] = linter.lint_file_diff( - str(tmp_path / 'old.py'), - str(tmp_path / 'new.py'), - ) - print(result) - assert len(result) == 1 - assert ( - result[0].line == 8 - and result[0].column == 9 - and result[0].message == "F821 undefined name 'ANOTHER_UNDEFINED_VARIABLE'" - ) - - -def test_lint_with_diff_insert(tmp_path): - with open(tmp_path / 'old.py', 'w') as f: - f.write(OLD_CONTENT) - with open(tmp_path / 'new.py', 'w') as f: - f.write(NEW_CONTENT_V2) - - linter = DefaultLinter() - result: list[LintResult] = linter.lint_file_diff( - str(tmp_path / 'old.py'), - str(tmp_path / 'new.py'), - ) - assert len(result) == 1 - assert ( - result[0].line == 5 - and result[0].column == 9 - and result[0].message == "F821 undefined name 'ANOTHER_UNDEFINED_VARIABLE'" - ) - - -def test_lint_with_multiple_changes_and_errors(tmp_path): - old_content = """ -def foo(): - print("Hello, World!") - x = 10 -foo() -""" - new_content = """ -def foo(): - print("Hello, World!") - x = UNDEFINED_VARIABLE - y = 20 - -def bar(): - z = ANOTHER_UNDEFINED_VARIABLE - return z + 1 - -foo() -bar() -""" - with open(tmp_path / 'old.py', 'w') as f: - f.write(old_content) - with open(tmp_path / 'new.py', 'w') as f: - f.write(new_content) - - linter = DefaultLinter() - result: list[LintResult] = linter.lint_file_diff( - str(tmp_path / 'old.py'), - str(tmp_path / 'new.py'), - ) - assert len(result) == 2 - assert ( - result[0].line == 4 - and result[0].column == 9 - and result[0].message == "F821 undefined name 'UNDEFINED_VARIABLE'" - ) - assert ( - result[1].line == 8 - and result[1].column == 9 - and result[1].message == "F821 undefined name 'ANOTHER_UNDEFINED_VARIABLE'" - ) - - -def test_lint_with_introduced_and_fixed_errors(tmp_path): - old_content = """ -x = UNDEFINED_VARIABLE -y = 10 -""" - new_content = """ -x = 5 -y = ANOTHER_UNDEFINED_VARIABLE -z = UNDEFINED_VARIABLE -""" - with open(tmp_path / 'old.py', 'w') as f: - f.write(old_content) - with open(tmp_path / 'new.py', 'w') as f: - f.write(new_content) - - linter = DefaultLinter() - result: list[LintResult] = linter.lint_file_diff( - str(tmp_path / 'old.py'), - str(tmp_path / 'new.py'), - ) - assert len(result) == 2 - assert ( - result[0].line == 3 - and result[0].column == 5 - and result[0].message == "F821 undefined name 'ANOTHER_UNDEFINED_VARIABLE'" - ) - assert ( - result[1].line == 4 - and result[1].column == 5 - and result[1].message == "F821 undefined name 'UNDEFINED_VARIABLE'" - ) - - -def test_lint_with_multiline_changes(tmp_path): - old_content = """ -def complex_function(a, b, c): - return (a + - b + - c) -""" - new_content = """ -def complex_function(a, b, c): - return (a + - UNDEFINED_VARIABLE + - b + - c) -""" - with open(tmp_path / 'old.py', 'w') as f: - f.write(old_content) - with open(tmp_path / 'new.py', 'w') as f: - f.write(new_content) - - linter = DefaultLinter() - result: list[LintResult] = linter.lint_file_diff( - str(tmp_path / 'old.py'), - str(tmp_path / 'new.py'), - ) - assert len(result) == 1 - assert ( - result[0].line == 4 - and result[0].column == 13 - and result[0].message == "F821 undefined name 'UNDEFINED_VARIABLE'" - ) - - -def test_lint_with_syntax_error(tmp_path): - old_content = """ -def foo(): - print("Hello, World!") -""" - new_content = """ -def foo(): - print("Hello, World!" -""" - with open(tmp_path / 'old.py', 'w') as f: - f.write(old_content) - with open(tmp_path / 'new.py', 'w') as f: - f.write(new_content) - - linter = DefaultLinter() - result: list[LintResult] = linter.lint_file_diff( - str(tmp_path / 'old.py'), - str(tmp_path / 'new.py'), - ) - assert len(result) == 1 - assert ( - result[0].line == 3 - and result[0].column == 11 - and result[0].message == "E999 SyntaxError: '(' was never closed" - ) - - -def test_lint_with_docstring_changes(tmp_path): - old_content = ''' -def foo(): - """This is a function.""" - print("Hello, World!") -''' - new_content = ''' -def foo(): - """ - This is a function. - It now has a multi-line docstring with an UNDEFINED_VARIABLE. - """ - print("Hello, World!") -''' - with open(tmp_path / 'old.py', 'w') as f: - f.write(old_content) - with open(tmp_path / 'new.py', 'w') as f: - f.write(new_content) - - linter = DefaultLinter() - result: list[LintResult] = linter.lint_file_diff( - str(tmp_path / 'old.py'), - str(tmp_path / 'new.py'), - ) - assert len(result) == 0 # Linter should ignore changes in docstrings - - -def test_lint_with_multiple_errors_on_same_line(tmp_path): - old_content = """ -def foo(): - print("Hello, World!") - x = 10 -foo() -""" - new_content = """ -def foo(): - print("Hello, World!") - x = UNDEFINED_VARIABLE + ANOTHER_UNDEFINED_VARIABLE -foo() -""" - with open(tmp_path / 'old.py', 'w') as f: - f.write(old_content) - with open(tmp_path / 'new.py', 'w') as f: - f.write(new_content) - - linter = DefaultLinter() - result: list[LintResult] = linter.lint_file_diff( - str(tmp_path / 'old.py'), - str(tmp_path / 'new.py'), - ) - print(result) - assert len(result) == 2 - assert ( - result[0].line == 4 - and result[0].column == 9 - and result[0].message == "F821 undefined name 'UNDEFINED_VARIABLE'" - ) - assert ( - result[1].line == 4 - and result[1].column == 30 - and result[1].message == "F821 undefined name 'ANOTHER_UNDEFINED_VARIABLE'" - ) - - -def test_parse_diff_with_empty_patch(): - diff_patch = '' - changes = parse_diff(diff_patch) - assert len(changes) == 0 - - -def test_lint_file_diff_ignore_existing_errors(tmp_path): - """ - Make sure we allow edits as long as it does not introduce new errors. In other - words, we don't care about existing linting errors. Although they might be - real syntax issues, sometimes they are just false positives, or errors that - we don't care about. - """ - content = """def some_valid_but_weird_function(): - # this function is legitimate, yet static analysis tools like flake8 - # reports 'F821 undefined name' - if 'variable' in locals(): - print(variable) -def some_wrong_but_unused_function(): - # this function has a linting error, but it is not modified by us, and - # who knows, this function might be completely dead code - x = 1 -def sum(a, b): - return a - b -""" - new_content = content.replace(' return a - b', ' return a + b') - temp_file_old_path = tmp_path / 'problematic-file-test.py' - temp_file_old_path.write_text(content) - temp_file_new_path = tmp_path / 'problematic-file-test-new.py' - temp_file_new_path.write_text(new_content) - - linter = DefaultLinter() - result: list[LintResult] = linter.lint_file_diff( - str(temp_file_old_path), - str(temp_file_new_path), - ) - assert len(result) == 0 # no new errors introduced - - -def test_lint_file_diff_catch_new_errors_in_edits(tmp_path): - """ - Make sure we catch new linting errors in our edit chunk, and at the same - time, ignore old linting errors (in this case, the old linting error is - a false positive) - """ - content = """def some_valid_but_weird_function(): - # this function is legitimate, yet static analysis tools like flake8 - # reports 'F821 undefined name' - if 'variable' in locals(): - print(variable) -def sum(a, b): - return a - b -""" - - temp_file_old_path = tmp_path / 'problematic-file-test.py' - temp_file_old_path.write_text(content) - new_content = content.replace(' return a - b', ' return a + variable') - temp_file_new_path = tmp_path / 'problematic-file-test-new.py' - temp_file_new_path.write_text(new_content) - - linter = DefaultLinter() - result: list[LintResult] = linter.lint_file_diff( - str(temp_file_old_path), - str(temp_file_new_path), - ) - print(result) - assert len(result) == 1 - assert ( - result[0].line == 7 - and result[0].column == 16 - and result[0].message == "F821 undefined name 'variable'" - ) - - -def test_lint_file_diff_catch_new_errors_outside_edits(tmp_path): - """ - Make sure we catch new linting errors induced by our edits, even - though the error itself is not in the edit chunk - """ - content = """def valid_func1(): - print(my_sum(1, 2)) -def my_sum(a, b): - return a - b -def valid_func2(): - print(my_sum(0, 0)) -""" - # Add 100 lines of invalid code, which linter shall ignore - # because they are not being edited. For testing purpose, we - # must add these existing linting errors, otherwise the pre-edit - # linting would pass, and thus there won't be any comparison - # between pre-edit and post-edit linting. - for _ in range(100): - content += '\ninvalid_func()' - - temp_file_old_path = tmp_path / 'problematic-file-test.py' - temp_file_old_path.write_text(content) - - new_content = content.replace('def my_sum(a, b):', 'def my_sum2(a, b):') - temp_file_new_path = tmp_path / 'problematic-file-test-new.py' - temp_file_new_path.write_text(new_content) - - linter = DefaultLinter() - result: list[LintResult] = linter.lint_file_diff( - str(temp_file_old_path), - str(temp_file_new_path), - ) - assert len(result) == 2 - assert ( - result[0].line == 2 - and result[0].column == 11 - and result[0].message == "F821 undefined name 'my_sum'" - ) - assert ( - result[1].line == 6 - and result[1].column == 11 - and result[1].message == "F821 undefined name 'my_sum'" - ) diff --git a/tests/unit/linters/test_python_linter.py b/tests/unit/linters/test_python_linter.py deleted file mode 100644 index 40aed81ec3f3..000000000000 --- a/tests/unit/linters/test_python_linter.py +++ /dev/null @@ -1,84 +0,0 @@ -from openhands.linter import DefaultLinter, LintResult -from openhands.linter.languages.python import ( - PythonLinter, - flake_lint, - python_compile_lint, -) - - -def test_wrongly_indented_py_file(wrongly_indented_py_file): - # Test Python linter - linter = PythonLinter() - assert '.py' in linter.supported_extensions - result = linter.lint(wrongly_indented_py_file) - print(result) - assert isinstance(result, list) and len(result) == 1 - assert result[0] == LintResult( - file=wrongly_indented_py_file, - line=2, - column=5, - message='E999 IndentationError: unexpected indent', - ) - print(result[0].visualize()) - assert result[0].visualize() == ( - '1|\n' - '\033[91m2| def foo():\033[0m\n' - ' ^ ERROR HERE: E999 IndentationError: unexpected indent\n' - '3| print("Hello, World!")\n' - '4|' - ) - - # General linter should have same result as Python linter - # bc it uses PythonLinter under the hood - general_linter = DefaultLinter() - assert '.py' in general_linter.supported_extensions - result = general_linter.lint(wrongly_indented_py_file) - assert result == linter.lint(wrongly_indented_py_file) - - # Test flake8_lint - assert result == flake_lint(wrongly_indented_py_file) - - # Test python_compile_lint - compile_result = python_compile_lint(wrongly_indented_py_file) - assert isinstance(compile_result, list) and len(compile_result) == 1 - assert compile_result[0] == LintResult( - file=wrongly_indented_py_file, line=2, column=4, message='unexpected indent' - ) - - -def test_simple_correct_py_file(simple_correct_py_file): - linter = PythonLinter() - assert '.py' in linter.supported_extensions - result = linter.lint(simple_correct_py_file) - assert result == [] - - general_linter = DefaultLinter() - assert '.py' in general_linter.supported_extensions - result = general_linter.lint(simple_correct_py_file) - assert result == linter.lint(simple_correct_py_file) - - # Test python_compile_lint - compile_result = python_compile_lint(simple_correct_py_file) - assert compile_result == [] - - # Test flake_lint - flake_result = flake_lint(simple_correct_py_file) - assert flake_result == [] - - -def test_simple_correct_py_func_def(simple_correct_py_func_def): - linter = PythonLinter() - result = linter.lint(simple_correct_py_func_def) - assert result == [] - - general_linter = DefaultLinter() - assert '.py' in general_linter.supported_extensions - result = general_linter.lint(simple_correct_py_func_def) - assert result == linter.lint(simple_correct_py_func_def) - - # Test flake_lint - assert result == flake_lint(simple_correct_py_func_def) - - # Test python_compile_lint - compile_result = python_compile_lint(simple_correct_py_func_def) - assert compile_result == [] diff --git a/tests/unit/linters/test_treesitter_linter.py b/tests/unit/linters/test_treesitter_linter.py deleted file mode 100644 index 195a48bf3632..000000000000 --- a/tests/unit/linters/test_treesitter_linter.py +++ /dev/null @@ -1,113 +0,0 @@ -from openhands.linter import DefaultLinter, LintResult -from openhands.linter.languages.treesitter import TreesitterBasicLinter - - -def test_syntax_error_py_file(syntax_error_py_file): - linter = TreesitterBasicLinter() - result = linter.lint(syntax_error_py_file) - print(result) - assert isinstance(result, list) and len(result) == 1 - assert result[0] == LintResult( - file=syntax_error_py_file, - line=5, - column=5, - message='Syntax error', - ) - - assert ( - result[0].visualize() - == ( - '2| def foo():\n' - '3| print("Hello, World!")\n' - '4| print("Wrong indent")\n' - '\033[91m5| foo(\033[0m\n' # color red - ' ^ ERROR HERE: Syntax error\n' - '6|' - ) - ) - print(result[0].visualize()) - - general_linter = DefaultLinter() - general_result = general_linter.lint(syntax_error_py_file) - # NOTE: general linter returns different result - # because it uses flake8 first, which is different from treesitter - assert general_result != result - - -def test_simple_correct_ruby_file(simple_correct_ruby_file): - linter = TreesitterBasicLinter() - result = linter.lint(simple_correct_ruby_file) - assert isinstance(result, list) and len(result) == 0 - - # Test that the general linter also returns the same result - general_linter = DefaultLinter() - general_result = general_linter.lint(simple_correct_ruby_file) - assert general_result == result - - -def test_simple_incorrect_ruby_file(simple_incorrect_ruby_file): - linter = TreesitterBasicLinter() - result = linter.lint(simple_incorrect_ruby_file) - print(result) - assert isinstance(result, list) and len(result) == 2 - assert result[0] == LintResult( - file=simple_incorrect_ruby_file, - line=1, - column=1, - message='Syntax error', - ) - print(result[0].visualize()) - assert ( - result[0].visualize() - == ( - '\033[91m1|def foo():\033[0m\n' # color red - ' ^ ERROR HERE: Syntax error\n' - '2| print("Hello, World!")\n' - '3|foo()' - ) - ) - assert result[1] == LintResult( - file=simple_incorrect_ruby_file, - line=1, - column=10, - message='Syntax error', - ) - print(result[1].visualize()) - assert ( - result[1].visualize() - == ( - '\033[91m1|def foo():\033[0m\n' # color red - ' ^ ERROR HERE: Syntax error\n' - '2| print("Hello, World!")\n' - '3|foo()' - ) - ) - - # Test that the general linter also returns the same result - general_linter = DefaultLinter() - general_result = general_linter.lint(simple_incorrect_ruby_file) - assert general_result == result - - -def test_parenthesis_incorrect_ruby_file(parenthesis_incorrect_ruby_file): - linter = TreesitterBasicLinter() - result = linter.lint(parenthesis_incorrect_ruby_file) - print(result) - assert isinstance(result, list) and len(result) == 1 - assert result[0] == LintResult( - file=parenthesis_incorrect_ruby_file, - line=1, - column=1, - message='Syntax error', - ) - print(result[0].visualize()) - assert result[0].visualize() == ( - '\033[91m1|def print_hello_world()\033[0m\n' - ' ^ ERROR HERE: Syntax error\n' - "2| puts 'Hello World'" - ) - - # Test that the general linter also returns the same result - general_linter = DefaultLinter() - general_result = general_linter.lint(parenthesis_incorrect_ruby_file) - assert general_result == result diff --git a/tests/unit/linters/test_visualize.py b/tests/unit/linters/test_visualize.py deleted file mode 100644 index e8232afd0117..000000000000 --- a/tests/unit/linters/test_visualize.py +++ /dev/null @@ -1,86 +0,0 @@ -from unittest.mock import mock_open, patch - -import pytest - -from openhands.linter.base import LintResult - - -@pytest.fixture -def mock_file_content(): - return '\n'.join([f'Line {i}' for i in range(1, 21)]) - - -def test_visualize_standard_case(mock_file_content): - lint_result = LintResult( - file='test_file.py', line=10, column=5, message='Test error message' - ) - - with patch('builtins.open', mock_open(read_data=mock_file_content)): - result = lint_result.visualize(half_window=3) - - expected_output = ( - " 7|Line 7\n" - " 8|Line 8\n" - " 9|Line 9\n" - "\033[91m10|Line 10\033[0m\n" - f" {' ' * lint_result.column}^ ERROR HERE: Test error message\n" - "11|Line 11\n" - "12|Line 12\n" - "13|Line 13" - ) - - assert result == expected_output - - -def test_visualize_small_window(mock_file_content): - lint_result = LintResult( - file='test_file.py', line=10, column=5, message='Test error message' - ) - - with patch('builtins.open', mock_open(read_data=mock_file_content)): - result = lint_result.visualize(half_window=1) - - expected_output = ( - " 9|Line 9\n" - "\033[91m10|Line 10\033[0m\n" - f" {' ' * lint_result.column}^ ERROR HERE: Test error message\n" - "11|Line 11" - ) - - assert result == expected_output - - -def test_visualize_error_at_start(mock_file_content): - lint_result = LintResult( - file='test_file.py', line=1, column=3, message='Start error' - ) - - with patch('builtins.open', mock_open(read_data=mock_file_content)): - result = lint_result.visualize(half_window=2) - - expected_output = ( - "\033[91m 1|Line 1\033[0m\n" - f" {' ' * lint_result.column}^ ERROR HERE: Start error\n" - " 2|Line 2\n" - " 3|Line 3" - ) - - assert result == expected_output - - -def test_visualize_error_at_end(mock_file_content): - lint_result = LintResult( - file='test_file.py', line=20, column=1, message='End error' - ) - - with patch('builtins.open', mock_open(read_data=mock_file_content)): - result = lint_result.visualize(half_window=2) - - expected_output = ( - "18|Line 18\n" - "19|Line 19\n" - "\033[91m20|Line 20\033[0m\n" - f" {' ' * lint_result.column}^ ERROR HERE: End error" - ) - - assert result == expected_output