Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feat] Add codecov client #1169

Merged
merged 18 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,8 @@ GITHUB_TOKEN=... # Or use GIHUB_PRIVATE_KEY and GITHUB_APP_ID
NO_SENTRY_INTEGRATION=... # Set this to 1 in develop mode to ignore Local Sentry Integration
NO_REAL_MODELS=... # Set this to 1 in development to ignore real models and use stubs

## Required for Codecov client
CODECOV_SUPER_TOKEN=...

RPC_SHARED_SECRET="seers-also-very-long-value-haha" # Match with SEER_RPC_SHARED_SECRET=[""] in sentry.conf.py
SBX_PROJECT=eng-dev-sbx--XXX # If using push-image and https://github.com/getsentry/terraform-sandboxes.private
Empty file added src/integrations/__init__.py
Empty file.
Empty file.
36 changes: 36 additions & 0 deletions src/integrations/codecov/codecov_client.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import requests

from seer.configuration import AppConfig
from seer.dependency_injection import inject, injected

class CodecovClient:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is totes fine for now, but in the future, using instance methods makes it easier to potentially subclass for test mock purposes.

@dataclasses.dataclass
class CodecovClient
    config: AppConfig = injected
    ...

class MockCodecovClient(CodecovClient):
    ...

This is a minor nit that doesn't need to be addresses here, but food for thought on future design.

@staticmethod
@inject
def fetch_coverage(owner_username, repo_name, pullid, config: AppConfig = injected):
token = config.CODECOV_SUPER_TOKEN
url = f"https://api.codecov.io/api/v2/github/{owner_username}/repos/{repo_name}/pulls/{pullid}"
headers = {"Authorization": f"Bearer {token}", "Accept": "application/json"}
response = requests.get(url, headers=headers)
if response.status_code == 200:
return response.text
else:
return None

@staticmethod
@inject
def fetch_test_results_for_commit(
owner_username, repo_name, latest_commit_sha, config: AppConfig = injected
):
token = config.CODECOV_SUPER_TOKEN
url = f"https://api.codecov.io/api/v2/github/{owner_username}/repos/{repo_name}/test-results?commit_id={latest_commit_sha}&outcome=failure"
headers = {
"Authorization": f"Bearer {token}",
"Accept": "application/json",
}
response = requests.get(url, headers=headers)
if response.status_code == 200:
if response.json()["count"] == 0:
return None
return response.text
else:
return None
14 changes: 11 additions & 3 deletions src/seer/automation/codebase/repo_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ def get_repo_app_permissions(
@inject
def get_github_token_auth(config: AppConfig = injected) -> Auth.Token | None:
github_token = config.GITHUB_TOKEN

if github_token is None:
return None

Expand All @@ -62,7 +61,6 @@ def get_write_app_credentials(config: AppConfig = injected) -> tuple[int | str |
private_key = config.GITHUB_PRIVATE_KEY

if not app_id or not private_key:

return None, None

return app_id, private_key
Expand Down Expand Up @@ -449,5 +447,15 @@ def get_pr_diff_content(self, pr_url: str) -> str:
data = requests.get(pr_url, headers=headers)

data.raise_for_status() # Raise an exception for HTTP errors

return data.text

def get_pr_head_sha(self, pr_url: str) -> str:
requester = self.repo._requester
headers = {
"Authorization": f"{requester.auth.token_type} {requester.auth.token}", # type: ignore
"Accept": "application/vnd.github.raw+json",
}

data = requests.get(pr_url, headers=headers)
data.raise_for_status() # Raise an exception for HTTP errors
return data.json()["head"]["sha"]
3 changes: 2 additions & 1 deletion src/seer/automation/codegen/models.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import datetime
from enum import Enum
from typing import Any, Dict, Optional

from pydantic import BaseModel, Field

Expand Down Expand Up @@ -45,7 +46,7 @@ def mark_updated(self):

class CodeUnitTestRequest(BaseComponentRequest):
diff: str

codecov_client_params: dict = Field(default_factory=dict)

class CodegenUnitTestsResponse(BaseModel):
run_id: int
Expand Down
39 changes: 33 additions & 6 deletions src/seer/automation/codegen/prompts.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ def format_system_msg():
)

@staticmethod
def format_plan_step_msg(diff_str: str):
return textwrap.dedent(
def format_plan_step_msg(
diff_str: str,
has_coverage_info: str | None = None,
has_test_result_info: str | None = None,
):
base_msg = textwrap.dedent(
"""\
You are given the below code changes as a diff:
{diff_str}
Expand All @@ -34,14 +38,37 @@ def format_plan_step_msg(diff_str: str):
# Guidelines:
- No placeholders are allowed, the unit test must be clear and detailed.
- Make sure you use the tools provided to look through the codebase and at the files you are changing before outputting your suggested fix.
- The unit tests must be comprehensive. Do not provide temporary examples, placeholders or incomplete ones.
- The unit tests must be comprehensive. Do not provide temporary examples, placeholders, or incomplete ones.
- In your suggested unit tests, whenever you are providing code, provide explicit diffs to show the exact changes that need to be made.
- All your changes should be in test files.
- EVERY TIME before you use a tool, think step-by-step each time before using the tools provided to you.
- You also MUST think step-by-step before giving the final answer."""
).format(
diff_str=diff_str,
)
).format(diff_str=diff_str)

if has_coverage_info:
coverage_info_msg = textwrap.dedent(
"""\
You are also given the following code coverage information for the current diff as a JSON object:
{coverage_info_str}

Remember, the goal is not just to improve coverage numbers but to verify the behavior of the code meaningfully, focusing on the recent changes.
Integrate this information with your diff analysis to provide a comprehensive and targeted testing strategy.
"""
).format(coverage_info_str=has_coverage_info)
base_msg += "\n\n" + coverage_info_msg

if has_test_result_info:
test_result_info_msg = textwrap.dedent(
"""\
You are provided with the following test result data for existing tests related to the diff:
{test_result_data}

Use this information to enhance your test creation strategy, ensuring new tests reinforce areas of failure and improve overall test suite effectiveness in the context of the introduced changes.
"""
).format(test_result_data=has_test_result_info)
base_msg += "\n\n" + test_result_info_msg

return base_msg

@staticmethod
def format_find_unit_test_pattern_step_msg(diff_str: str):
Expand Down
1 change: 0 additions & 1 deletion src/seer/automation/codegen/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ def codegen_unittest(request: CodegenUnitTestsRequest):
state = create_initial_unittest_run(request)

cur_state = state.get()

# Process has no further work.
# if cur_state.status in CodegenStatus.terminal():
# logger.warning(f"Ignoring job, state {cur_state.status}")
Expand Down
23 changes: 21 additions & 2 deletions src/seer/automation/codegen/unit_test_coding_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from seer.automation.component import BaseComponent
from seer.automation.models import FileChange
from seer.automation.utils import escape_multi_xml, extract_text_inside_tags
from integrations.codecov.codecov_client import CodecovClient

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -50,6 +51,20 @@ def invoke(self, request: CodeUnitTestRequest) -> CodeUnitTestOutput | None:
),
)

codecov_client_params = request.codecov_client_params

code_coverage_data = CodecovClient.fetch_coverage(
repo_name=codecov_client_params["repo_name"],
pullid=codecov_client_params["pullid"],
owner_username=codecov_client_params["owner_username"],
)

test_result_data = CodecovClient.fetch_test_results_for_commit(
repo_name=codecov_client_params["repo_name"],
owner_username=codecov_client_params["owner_username"],
latest_commit_sha=codecov_client_params["head_sha"],
)

existing_test_design_response = self._get_test_design_summary(
agent=agent,
prompt=CodingUnitTestPrompts.format_find_unit_test_pattern_step_msg(
Expand All @@ -58,7 +73,12 @@ def invoke(self, request: CodeUnitTestRequest) -> CodeUnitTestOutput | None:
)

self._get_plan(
agent=agent, prompt=CodingUnitTestPrompts.format_plan_step_msg(diff_str=request.diff)
agent=agent,
prompt=CodingUnitTestPrompts.format_plan_step_msg(
diff_str=request.diff,
has_coverage_info=code_coverage_data,
has_test_result_info=test_result_data,
),
)

final_response = self._generate_tests(
Expand All @@ -70,7 +90,6 @@ def invoke(self, request: CodeUnitTestRequest) -> CodeUnitTestOutput | None:

if not final_response:
return None

plan_steps_content = extract_text_inside_tags(final_response, "plan_steps")

if len(plan_steps_content) == 0:
Expand Down
13 changes: 11 additions & 2 deletions src/seer/automation/codegen/unittest_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,22 @@ def _invoke(self, **kwargs):

repo_client = self.context.get_repo_client()
pr = repo_client.repo.get_pull(self.request.pr_id)

diff_content = repo_client.get_pr_diff_content(pr.url)

latest_commit_sha = repo_client.get_pr_head_sha(pr.url)

codecov_client_params = {
"repo_name": self.request.repo_definition.name,
"pullid": self.request.pr_id,
"owner_username": self.request.repo_definition.owner,
"head_sha": latest_commit_sha,
}

unittest_output = UnitTestCodingComponent(self.context).invoke(
CodeUnitTestRequest(
diff=diff_content,
)
codecov_client_params=codecov_client_params,
),
)

if unittest_output:
Expand Down
57 changes: 57 additions & 0 deletions tests/automation/codegen/test_unit_test_step.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import unittest
from unittest.mock import patch, MagicMock

from seer.automation.codegen.models import CodeUnitTestRequest
from seer.automation.codegen.unittest_step import UnittestStep, UnittestStepRequest
from seer.automation.models import RepoDefinition

import unittest
from unittest.mock import MagicMock

from seer.automation.models import RepoDefinition

class TestUnittestStep(unittest.TestCase):
@patch("seer.automation.codegen.unit_test_coding_component.UnitTestCodingComponent.invoke")
@patch("seer.automation.pipeline.PipelineStep", new_callable=MagicMock)
@patch("seer.automation.codegen.step.CodegenStep._instantiate_context", new_callable=MagicMock)
def test_invoke_happy_path(self,
mock_instantiate_context,
_,
mock_invoke_unit_test_component):

mock_repo_client = MagicMock()
mock_pr = MagicMock()
mock_diff_content = "diff content"
mock_latest_commit_sha = "latest_commit_sha"
mock_context = MagicMock()
mock_context.get_repo_client.return_value = mock_repo_client
mock_repo_client.repo.get_pull.return_value = mock_pr
mock_repo_client.get_pr_diff_content.return_value = mock_diff_content
mock_repo_client.get_pr_head_sha.return_value = mock_latest_commit_sha

request_data = {
"run_id": 1,
"pr_id": 123,
"repo_definition": RepoDefinition(name="repo1", owner="owner1", provider="github", external_id="123123")
}
request = UnittestStepRequest(**request_data)
step = UnittestStep(request=request)
step.context = mock_context

step.invoke()

mock_context.get_repo_client.assert_called_once()
mock_repo_client.repo.get_pull.assert_called_once_with(request.pr_id)
mock_repo_client.get_pr_diff_content.assert_called_once_with(mock_pr.url)
mock_repo_client.get_pr_head_sha.assert_called_once_with(mock_pr.url)

actual_request = mock_invoke_unit_test_component.call_args[0][0]

assert isinstance(actual_request, CodeUnitTestRequest)
assert actual_request.diff == mock_diff_content
assert actual_request.codecov_client_params == {
"repo_name": request.repo_definition.name,
"pullid": request.pr_id,
"owner_username": request.repo_definition.owner,
"head_sha": mock_latest_commit_sha,
}
75 changes: 75 additions & 0 deletions tests/integrations/test_codecov_client.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import unittest
from unittest.mock import patch, MagicMock
from integrations.codecov.codecov_client import CodecovClient
from seer.configuration import AppConfig


class MockConfig:
CODECOV_SUPER_TOKEN = "test_token"


class TestCodecovClient(unittest.TestCase):
@patch("integrations.codecov.codecov_client.requests.get")
def test_fetch_coverage_success(self, mock_get):
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.text = "Mock coverage data"
mock_get.return_value = mock_response

result = CodecovClient.fetch_coverage("owner", "repo", "123", config=MockConfig())

self.assertEqual(result, "Mock coverage data")
mock_get.assert_called_once()

@patch("integrations.codecov.codecov_client.requests.get")
def test_fetch_coverage_failure(self, mock_get):
mock_response = MagicMock()
mock_response.status_code = 404
mock_get.return_value = mock_response

result = CodecovClient.fetch_coverage("owner", "repo", "123", config=MockConfig())

self.assertIsNone(result)
mock_get.assert_called_once()

@patch("integrations.codecov.codecov_client.requests.get")
def test_fetch_test_results_for_commit_success_with_results(self, mock_get):
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.text = "Mock test results data"
mock_response.json.return_value = {"count": 1}
mock_get.return_value = mock_response

result = CodecovClient.fetch_test_results_for_commit(
"owner", "repo", "commit_sha", config=MockConfig()
)

self.assertEqual(result, "Mock test results data")
mock_get.assert_called_once()

@patch("integrations.codecov.codecov_client.requests.get")
def test_fetch_test_results_for_commit_success_no_results(self, mock_get):
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.json.return_value = {"count": 0}
mock_get.return_value = mock_response

result = CodecovClient.fetch_test_results_for_commit(
"owner", "repo", "commit_sha", config=MockConfig()
)

self.assertIsNone(result)
mock_get.assert_called_once()

@patch("integrations.codecov.codecov_client.requests.get")
def test_fetch_test_results_for_commit_failure(self, mock_get):
mock_response = MagicMock()
mock_response.status_code = 500
mock_get.return_value = mock_response

result = CodecovClient.fetch_test_results_for_commit(
"owner", "repo", "commit_sha", config=MockConfig()
)

self.assertIsNone(result)
mock_get.assert_called_once()
Loading