-
Notifications
You must be signed in to change notification settings - Fork 326
test: consolidate test utilities and improve test structure #423
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
Closed
Closed
Changes from 7 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
d2d09b8
refactor: consolidate test utilities and improve test structure
ahoblitz ee76e99
mr comments
ahoblitz 9c976f6
revert: Revert "chore(gRPC): Update a2a.proto to include metadata on …
a2a-bot 8a6d1c7
chore: merge updates and mr comments
ahoblitz b0d3d7a
Merge branch 'main' into test/refactor-tests
ahoblitz 7a0b80a
Merge branch 'main' into test/refactor-tests
ahoblitz 999096a
Merge branch 'main' into test/refactor-tests
ahoblitz c8769e2
Merge branch 'main' into test/refactor-tests - Resolved conflicts in …
ahoblitz 1ad01bd
patch tests
ahoblitz 8e9928f
Merge branch 'main' into test/refactor-tests
ahoblitz b158e42
Merge branch 'main' into test/refactor-tests
ahoblitz 4f86b2c
Merge branch 'main' into test/refactor-tests
ahoblitz 7a83c97
Merge branch 'main' into test/refactor-tests
holtskinner bd56631
Update tests/fixtures.py
ahoblitz 3af518a
Update tests/builders.py
ahoblitz 4928889
Update tests/builders.py
ahoblitz 2e0d293
Update tests/fixtures.py
ahoblitz 40656b2
Update tests/fixtures.py
ahoblitz c7f156d
ruff
ahoblitz 650f130
Merge branch 'main' into test/refactor-tests
ahoblitz 2aba8b8
linter
ahoblitz 76ea260
Merge branch 'test/refactor-tests' of github.com:ahoblitz/a2a-python …
ahoblitz 36321d9
Merge branch 'main' into test/refactor-tests
ahoblitz cd7121b
Merge branch 'main' into test/refactor-tests
ahoblitz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
Empty file.
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,210 @@ | ||
| from dataclasses import dataclass, field | ||
| from typing import Any | ||
|
|
||
| from a2a.types import ( | ||
| Artifact, | ||
| Message, | ||
| Part, | ||
| Role, | ||
| Task, | ||
| TaskArtifactUpdateEvent, | ||
| TaskState, | ||
| TaskStatus, | ||
| TaskStatusUpdateEvent, | ||
| TextPart, | ||
| ) | ||
|
|
||
|
|
||
| @dataclass | ||
| class TaskBuilder: | ||
| id: str = 'task-default' | ||
| context_id: str = 'context-default' | ||
| state: TaskState = TaskState.submitted | ||
| kind: str = 'task' | ||
| artifacts: list[Artifact] = field(default_factory=list) | ||
| history: list[Message] = field(default_factory=list) | ||
| metadata: dict[str, Any] = field(default_factory=dict) | ||
|
|
||
| def with_id(self, id: str) -> 'TaskBuilder': | ||
| self.id = id | ||
| return self | ||
ahoblitz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| def with_context_id(self, context_id: str) -> 'TaskBuilder': | ||
| self.context_id = context_id | ||
| return self | ||
|
|
||
| def with_state(self, state: TaskState) -> 'TaskBuilder': | ||
| self.state = state | ||
| return self | ||
|
|
||
| def with_metadata(self, **kwargs) -> 'TaskBuilder': | ||
| self.metadata.update(kwargs) | ||
| return self | ||
|
|
||
| def with_history(self, *messages: Message) -> 'TaskBuilder': | ||
| self.history.extend(messages) | ||
| return self | ||
|
|
||
| def with_artifacts(self, *artifacts: Artifact) -> 'TaskBuilder': | ||
| self.artifacts.extend(artifacts) | ||
| return self | ||
|
|
||
| def build(self) -> Task: | ||
| return Task( | ||
| id=self.id, | ||
| context_id=self.context_id, | ||
| status=TaskStatus(state=self.state), | ||
| kind=self.kind, | ||
| artifacts=self.artifacts if self.artifacts else None, | ||
| history=self.history if self.history else None, | ||
| metadata=self.metadata if self.metadata else None, | ||
| ) | ||
|
|
||
|
|
||
| @dataclass | ||
| class MessageBuilder: | ||
| role: Role = Role.user | ||
| text: str = 'default message' | ||
| message_id: str = 'msg-default' | ||
| task_id: str | None = None | ||
| context_id: str | None = None | ||
|
|
||
| def as_agent(self) -> 'MessageBuilder': | ||
| self.role = Role.agent | ||
| return self | ||
|
|
||
| def as_user(self) -> 'MessageBuilder': | ||
| self.role = Role.user | ||
| return self | ||
|
|
||
| def with_text(self, text: str) -> 'MessageBuilder': | ||
| self.text = text | ||
| return self | ||
|
|
||
| def with_id(self, message_id: str) -> 'MessageBuilder': | ||
| self.message_id = message_id | ||
| return self | ||
|
|
||
| def with_task_id(self, task_id: str) -> 'MessageBuilder': | ||
| self.task_id = task_id | ||
| return self | ||
|
|
||
| def with_context_id(self, context_id: str) -> 'MessageBuilder': | ||
| self.context_id = context_id | ||
| return self | ||
|
|
||
| def build(self) -> Message: | ||
| return Message( | ||
| role=self.role, | ||
| parts=[Part(TextPart(text=self.text))], | ||
| message_id=self.message_id, | ||
| task_id=self.task_id, | ||
| context_id=self.context_id, | ||
| ) | ||
|
|
||
|
|
||
| @dataclass | ||
| class ArtifactBuilder: | ||
| artifact_id: str = 'artifact-default' | ||
| name: str = 'default artifact' | ||
| text: str = 'default content' | ||
| description: str | None = None | ||
|
|
||
| def with_id(self, artifact_id: str) -> 'ArtifactBuilder': | ||
| self.artifact_id = artifact_id | ||
| return self | ||
|
|
||
| def with_name(self, name: str) -> 'ArtifactBuilder': | ||
| self.name = name | ||
| return self | ||
|
|
||
| def with_text(self, text: str) -> 'ArtifactBuilder': | ||
| self.text = text | ||
| return self | ||
|
|
||
| def with_description(self, description: str) -> 'ArtifactBuilder': | ||
| self.description = description | ||
| return self | ||
|
|
||
| def build(self) -> Artifact: | ||
| return Artifact( | ||
| artifact_id=self.artifact_id, | ||
| name=self.name, | ||
| parts=[Part(TextPart(text=self.text))], | ||
| description=self.description, | ||
| ) | ||
|
|
||
|
|
||
| @dataclass | ||
| class StatusUpdateEventBuilder: | ||
| task_id: str = 'task-default' | ||
| context_id: str = 'context-default' | ||
| state: TaskState = TaskState.working | ||
| message: Message | None = None | ||
| final: bool = False | ||
| metadata: dict[str, Any] = field(default_factory=dict) | ||
|
|
||
| def for_task(self, task_id: str) -> 'StatusUpdateEventBuilder': | ||
| self.task_id = task_id | ||
| return self | ||
|
|
||
| def with_state(self, state: TaskState) -> 'StatusUpdateEventBuilder': | ||
| self.state = state | ||
| return self | ||
|
|
||
| def with_message(self, message: Message) -> 'StatusUpdateEventBuilder': | ||
| self.message = message | ||
| return self | ||
|
|
||
| def as_final(self) -> 'StatusUpdateEventBuilder': | ||
| self.final = True | ||
| return self | ||
|
|
||
| def with_metadata(self, **kwargs) -> 'StatusUpdateEventBuilder': | ||
| self.metadata.update(kwargs) | ||
| return self | ||
|
|
||
| def build(self) -> TaskStatusUpdateEvent: | ||
| return TaskStatusUpdateEvent( | ||
| task_id=self.task_id, | ||
| context_id=self.context_id, | ||
| status=TaskStatus(state=self.state, message=self.message), | ||
| final=self.final, | ||
| metadata=self.metadata if self.metadata else None, | ||
| ) | ||
|
|
||
|
|
||
| @dataclass | ||
| class ArtifactUpdateEventBuilder: | ||
| task_id: str = 'task-default' | ||
| context_id: str = 'context-default' | ||
| artifact: Artifact | None = None | ||
| append: bool = False | ||
| last_chunk: bool = False | ||
|
|
||
| def for_task(self, task_id: str) -> 'ArtifactUpdateEventBuilder': | ||
| self.task_id = task_id | ||
| return self | ||
|
|
||
| def with_artifact(self, artifact: Artifact) -> 'ArtifactUpdateEventBuilder': | ||
| self.artifact = artifact | ||
| return self | ||
|
|
||
| def as_append(self) -> 'ArtifactUpdateEventBuilder': | ||
| self.append = True | ||
| return self | ||
|
|
||
| def as_last_chunk(self) -> 'ArtifactUpdateEventBuilder': | ||
| self.last_chunk = True | ||
| return self | ||
|
|
||
| def build(self) -> TaskArtifactUpdateEvent: | ||
| if not self.artifact: | ||
| self.artifact = ArtifactBuilder().build() | ||
| return TaskArtifactUpdateEvent( | ||
| task_id=self.task_id, | ||
| context_id=self.context_id, | ||
| artifact=self.artifact, | ||
| append=self.append, | ||
| last_chunk=self.last_chunk, | ||
| ) | ||
ahoblitz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These fixtures don't appear to be used anywhere and frankly I don't see a huge benefit in adding them. I'd even say seem to introduce unnecessary complexity. |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| import pytest | ||
|
|
||
| from a2a.server.tasks import TaskManager | ||
| from a2a.types import TaskState | ||
| from tests.builders import ( | ||
| ArtifactBuilder, | ||
| MessageBuilder, | ||
| TaskBuilder, | ||
| ) | ||
| from tests.test_doubles import ( | ||
| FakeHttpClient, | ||
| InMemoryTaskStore, | ||
| SpyEventQueue, | ||
| StubPushNotificationConfigStore, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def task_store(): | ||
| return InMemoryTaskStore() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def event_queue(): | ||
| return SpyEventQueue() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def push_config_store(): | ||
| return StubPushNotificationConfigStore() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def http_client(): | ||
| return FakeHttpClient() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def task_builder(): | ||
| return TaskBuilder() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def message_builder(): | ||
| return MessageBuilder() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def artifact_builder(): | ||
| return ArtifactBuilder() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def submitted_task(task_builder): | ||
| return task_builder.with_state(TaskState.submitted).build() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def working_task(task_builder): | ||
| return task_builder.with_state(TaskState.working).build() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def completed_task(task_builder): | ||
| return task_builder.with_state(TaskState.completed).build() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def task_with_history(task_builder, message_builder): | ||
| messages = [ | ||
| message_builder.as_user().with_text('Hello').build(), | ||
| message_builder.as_agent().with_text('Hi there!').build(), | ||
| ] | ||
| return task_builder.with_history(*messages).build() | ||
ahoblitz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| @pytest.fixture | ||
| def task_with_artifacts(task_builder, artifact_builder): | ||
| artifacts = [ | ||
| artifact_builder.with_id('art1').with_name('file.txt').build(), | ||
| artifact_builder.with_id('art2').with_name('data.json').build(), | ||
| ] | ||
| return task_builder.with_artifacts(*artifacts).build() | ||
ahoblitz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| @pytest.fixture | ||
| def task_manager(task_store): | ||
| return TaskManager( | ||
| task_id='task-123', | ||
| context_id='context-456', | ||
| task_store=task_store, | ||
| initial_message=None, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def task_manager_factory(task_store): | ||
| def factory(task_id=None, context_id=None, initial_message=None): | ||
| return TaskManager( | ||
| task_id=task_id, | ||
| context_id=context_id, | ||
| task_store=task_store, | ||
| initial_message=initial_message, | ||
| ) | ||
|
|
||
| return factory | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def populated_task_store(task_store, task_builder): | ||
| tasks = [ | ||
| task_builder.with_id('task-1').with_state(TaskState.submitted).build(), | ||
| task_builder.with_id('task-2').with_state(TaskState.working).build(), | ||
| task_builder.with_id('task-3').with_state(TaskState.completed).build(), | ||
| ] | ||
| for task in tasks: | ||
| task_store.set_task(task) | ||
| return task_store | ||
ahoblitz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this addition needed?