diff --git a/pyproject.toml b/pyproject.toml index a24fadd89..1cc9c8bbf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "GitAuto" -version = "1.6.3" +version = "1.6.6" requires-python = ">=3.14" dependencies = [ "annotated-doc==0.0.4", diff --git a/services/webhook/new_pr_handler.py b/services/webhook/new_pr_handler.py index a14490996..d8a64beda 100644 --- a/services/webhook/new_pr_handler.py +++ b/services/webhook/new_pr_handler.py @@ -498,21 +498,11 @@ async def handle_new_pr( user_input_obj["test_naming_convention"] = test_naming # Detect repo's test location convention (co-located, __tests__, separate dir) - # Skip auto-detection if user explicitly configured test file location - # Cross-ref: website repo app/dashboard/rules/config/structured-rules.ts testFileLocation options - structured_rules = repo_settings.get("structured_rules") if repo_settings else None - explicit_location = None - if isinstance(structured_rules, dict): - test_file_location = structured_rules.get("testFileLocation") - if test_file_location and test_file_location != "Auto-detect from repo": - explicit_location = test_file_location - if explicit_location: - user_input_obj["test_location_convention"] = explicit_location - logger.info("Using explicit test location setting: %s", explicit_location) - else: - test_location = detect_test_location_convention(clone_dir) - if test_location: - user_input_obj["test_location_convention"] = test_location + # Always auto-detect from the actual repo. Dashboard testFileLocation default ("Co-located with source") was overriding auto-detection for repos whose tests are actually in separate directories (e.g. test/specs/). + test_location = detect_test_location_convention(clone_dir) + if test_location: + user_input_obj["test_location_convention"] = test_location + logger.info("Auto-detected test location convention: %s", test_location) user_input = dumps(user_input_obj) messages: list[MessageParam] = [{"role": "user", "content": user_input}] diff --git a/services/webhook/test_new_pr_handler.py b/services/webhook/test_new_pr_handler.py index de8120dfc..39228ba18 100644 --- a/services/webhook/test_new_pr_handler.py +++ b/services/webhook/test_new_pr_handler.py @@ -2005,3 +2005,118 @@ async def test_many_test_files_include_paths_only_in_prompt( assert len(test_file_msgs) == 5 # read_local_file called: 1 for impl file + 5 for top test files = 6 assert mock_read_local_file.call_count == 6 + + +@pytest.mark.asyncio +@patch( + "services.webhook.new_pr_handler.run_subprocess", return_value=MagicMock(stdout="") +) +@patch("services.webhook.new_pr_handler.insert_credit") +@patch("services.webhook.new_pr_handler.should_bail", return_value=False) +@patch("services.webhook.new_pr_handler.create_empty_commit") +@patch("services.webhook.new_pr_handler.get_remote_file_content_by_url") +@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.slack_notify") +@patch("services.webhook.new_pr_handler.create_progress_bar") +@patch("services.webhook.new_pr_handler.update_usage") +@patch("services.webhook.new_pr_handler.update_comment") +@patch("services.webhook.new_pr_handler.chat_with_agent") +@patch("services.webhook.new_pr_handler.create_user_request") +@patch("services.webhook.new_pr_handler.read_local_file") +@patch("services.webhook.new_pr_handler.find_test_files") +@patch("services.webhook.new_pr_handler.clone_repo_and_install_dependencies") +@patch("services.webhook.new_pr_handler.ensure_node_packages") +@patch("services.webhook.new_pr_handler.get_owner") +@patch("services.webhook.new_pr_handler.create_comment") +@patch("services.webhook.new_pr_handler.check_availability") +@patch("services.webhook.new_pr_handler.render_text") +@patch("services.webhook.new_pr_handler.deconstruct_github_payload") +@patch("services.webhook.new_pr_handler.detect_test_location_convention") +async def test_auto_detect_location_ignores_dashboard_setting( + mock_detect_location, + mock_deconstruct_github_payload, + mock_render_text, + mock_check_availability, + mock_create_comment, + mock_get_owner, + mock_ensure_node_packages, + mock_prepare_repo, + mock_find_test_files, + mock_read_local_file, + mock_create_user_request, + mock_chat_with_agent, + mock_update_comment, + mock_update_usage, + mock_create_progress_bar, + mock_slack_notify, + mock_get_comments, + mock_get_remote_file_content_by_url, + mock_create_empty_commit, + mock_should_bail, + mock_insert_credit, + _mock_run_subprocess, +): + # Auto-detection returns "separate" even though dashboard says "Co-located" + mock_detect_location.return_value = ( + "separate test directory (e.g., test/specs/foo.spec.ts)" + ) + mock_deconstruct_github_payload.return_value = ( + { + **_get_base_args(), + "github_urls": {}, + "clone_url": "https://x-access-token:t@github.com/o/r.git", + "base_branch": "main", + }, + None, + ) + mock_render_text.return_value = "body" + mock_check_availability.return_value = { + "can_proceed": True, + "billing_type": "credit", + "credit_balance_usd": 50, + "user_message": "", + "log_message": "ok", + } + mock_create_comment.return_value = "https://api.github.com/comment/1" + mock_get_owner.return_value = { + "id": 456, + "credit_balance_usd": 100, + # Dashboard says "Co-located with source" but auto-detect says "separate" + "structured_rules": {"testFileLocation": "Co-located with source"}, + } + mock_create_user_request.return_value = 999 + mock_find_test_files.return_value = [] + mock_read_local_file.return_value = "function foo() {}" + mock_chat_with_agent.return_value = AgentResult( + messages=[], + token_input=10, + token_output=5, + is_completed=True, + completion_reason="", + p=0, + is_planned=False, + cost_usd=0.0, + ) + mock_update_comment.return_value = None + mock_update_usage.return_value = None + mock_create_progress_bar.return_value = "Progress: 0%" + mock_slack_notify.return_value = "thread_1" + mock_get_comments.return_value = [] + mock_get_remote_file_content_by_url.return_value = ("", "") + mock_create_empty_commit.return_value = None + mock_insert_credit.return_value = None + + payload = _get_test_payload() + await handle_new_pr(payload=payload, trigger="dashboard") + + # detect_test_location_convention must always be called (not skipped) + mock_detect_location.assert_called_once() + + # Verify auto-detected "separate" appears in user_input, not dashboard default + call_kwargs = mock_chat_with_agent.call_args.kwargs + messages = call_kwargs["messages"] + user_input = json.loads(messages[0]["content"]) + assert ( + user_input["test_location_convention"] + == "separate test directory (e.g., test/specs/foo.spec.ts)" + ) diff --git a/utils/files/detect_test_location_convention.py b/utils/files/detect_test_location_convention.py index 817ff571e..292ef971f 100644 --- a/utils/files/detect_test_location_convention.py +++ b/utils/files/detect_test_location_convention.py @@ -42,7 +42,7 @@ def detect_test_location_convention(clone_dir: str): return None dominant = max(counts, key=lambda k: counts[k]) - if counts[dominant] / total < 0.6: + if counts[dominant] / total < 0.8: logger.info( "No dominant test location convention (best=%s at %d/%d) in %s", dominant, diff --git a/utils/files/test_detect_test_location_convention.py b/utils/files/test_detect_test_location_convention.py index cae24abdb..8923cd86a 100644 --- a/utils/files/test_detect_test_location_convention.py +++ b/utils/files/test_detect_test_location_convention.py @@ -133,3 +133,37 @@ def test_spec_directory_counts_as_separate(): result = detect_test_location_convention(tmp) assert result is not None assert result.startswith("separate") + + +def test_majority_below_80_percent_returns_none(): + # 3 separate + 1 co-located = 75% separate, under 80% threshold + with tempfile.TemporaryDirectory() as tmp: + _create_files( + tmp, + [ + "src/utils/foo.test.ts", + "tests/test_bar.py", + "tests/test_baz.py", + "tests/test_qux.py", + ], + ) + result = detect_test_location_convention(tmp) + assert result is None + + +def test_exactly_80_percent_returns_dominant(): + # 4 separate + 1 co-located = 80% separate, meets threshold + with tempfile.TemporaryDirectory() as tmp: + _create_files( + tmp, + [ + "src/utils/foo.test.ts", + "tests/test_bar.py", + "tests/test_baz.py", + "tests/test_qux.py", + "tests/test_quux.py", + ], + ) + result = detect_test_location_convention(tmp) + assert result is not None + assert result.startswith("separate") diff --git a/utils/prompts/coding_standards.xml b/utils/prompts/coding_standards.xml index f84a62fb2..2c6580e98 100644 --- a/utils/prompts/coding_standards.xml +++ b/utils/prompts/coding_standards.xml @@ -27,9 +27,11 @@ - One test file per source file. NEVER create test_foo_new.py or test_foo_temp.py variants. If a test file already exists at ANY path, add tests there. - Co-location: test file in the SAME directory as source (e.g., foo.test.ts next to foo.ts). __tests__/ is NOT co-location. - If structured_repository_rules has testFileLocation set to co-locate, ALWAYS co-locate. Otherwise follow the repo's dominant pattern (80%+ majority). If mixed or no pattern, default to co-location. + One test file per source file. NEVER create test_foo_new.py or test_foo_temp.py variants. + Co-location is ideal: place test files in the SAME directory as the source file (e.g., foo.test.ts next to foo.ts). Exception: if "test_location_convention" says "separate" (80%+ of repo tests are in a dedicated test directory), follow that pattern instead. + If an existing test file is in a separate directory but the convention is NOT "separate": move it to co-location using move_file, then edit it there. + If 2+ test files exist for the same impl file (one co-located, one in a separate dir): keep the co-located one, merge test cases from the separate file into it, then delete the separate file. + "test_dir_prefixes" in user input only tells you where to FIND existing tests. It does not dictate where to CREATE new ones. diff --git a/uv.lock b/uv.lock index aaf45f8a9..c5bb6a8a1 100644 --- a/uv.lock +++ b/uv.lock @@ -596,7 +596,7 @@ wheels = [ [[package]] name = "gitauto" -version = "1.6.3" +version = "1.6.6" source = { virtual = "." } dependencies = [ { name = "annotated-doc" },