Skip to content

Commit 5253607

Browse files
authored
Merge branch '1.0-dev' into knap/722-google-rpc-status-errors
2 parents 27624a2 + c162c13 commit 5253607

File tree

12 files changed

+341
-200
lines changed

12 files changed

+341
-200
lines changed
Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
name: Post Coverage Comment
2+
3+
on:
4+
workflow_run:
5+
workflows: ["Run Unit Tests"]
6+
types:
7+
- completed
8+
9+
permissions:
10+
pull-requests: write
11+
actions: read
12+
13+
jobs:
14+
comment:
15+
runs-on: ubuntu-latest
16+
if: >
17+
github.event.workflow_run.event == 'pull_request' &&
18+
github.event.workflow_run.conclusion == 'success'
19+
steps:
20+
- name: Download Coverage Artifacts
21+
uses: actions/download-artifact@v4
22+
with:
23+
run-id: ${{ github.event.workflow_run.id }}
24+
github-token: ${{ secrets.A2A_BOT_PAT }}
25+
name: coverage-data
26+
27+
- name: Upload Coverage Report
28+
id: upload-report
29+
uses: actions/upload-artifact@v4
30+
with:
31+
name: coverage-report
32+
path: coverage/
33+
retention-days: 14
34+
35+
- name: Post Comment
36+
uses: actions/github-script@v6
37+
env:
38+
ARTIFACT_URL: ${{ steps.upload-report.outputs.artifact-url }}
39+
with:
40+
script: |
41+
const fs = require('fs');
42+
43+
const { owner, repo } = context.repo;
44+
const headSha = context.payload.workflow_run.head_commit.id;
45+
46+
const loadSummary = (path) => {
47+
try {
48+
const data = JSON.parse(fs.readFileSync(path, 'utf8'));
49+
// Map Python coverage.json format to expected internal summary format
50+
if (data.totals && data.files) {
51+
const summary = {
52+
total: {
53+
statements: { pct: data.totals.percent_covered }
54+
}
55+
};
56+
for (const [file, fileData] of Object.entries(data.files)) {
57+
// Python coverage uses absolute paths or relative to project root
58+
// We keep it as is for comparison
59+
summary[file] = {
60+
statements: { pct: fileData.summary.percent_covered }
61+
};
62+
}
63+
return summary;
64+
}
65+
return data;
66+
} catch (e) {
67+
console.log(`Could not read ${path}: ${e}`);
68+
return null;
69+
}
70+
};
71+
72+
const baseSummary = loadSummary('./coverage-base.json');
73+
const prSummary = loadSummary('./coverage-pr.json');
74+
75+
if (!baseSummary || !prSummary) {
76+
console.log("Missing coverage data, skipping comment.");
77+
return;
78+
}
79+
80+
let baseBranch = 'main';
81+
try {
82+
baseBranch = fs.readFileSync('./BASE_BRANCH', 'utf8').trim();
83+
} catch (e) {
84+
console.log("Could not read BASE_BRANCH, defaulting to main.");
85+
}
86+
87+
let markdown = `### 🧪 Code Coverage (vs \`${baseBranch}\`)\n\n`;
88+
89+
markdown += `[⬇️ **Download Full Report**](${process.env.ARTIFACT_URL})\n\n`;
90+
91+
const metric = 'statements';
92+
const getPct = (summaryItem, m) => summaryItem && summaryItem[m] ? Number(summaryItem[m].pct) : 0;
93+
94+
const formatDiff = (oldPct, newPct) => {
95+
const diff = (newPct - oldPct).toFixed(2);
96+
97+
let icon = '';
98+
if (diff > 0) icon = '🟢';
99+
else if (diff < 0) icon = '🔴';
100+
else icon = '⚪️';
101+
102+
const diffStr = diff > 0 ? `+${diff}%` : `${diff}%`;
103+
return `${icon} ${diffStr}`;
104+
};
105+
106+
const fileUrl = (path) => `https://github.com/${owner}/${repo}/blob/${headSha}/${path}`;
107+
108+
const allFiles = new Set([...Object.keys(baseSummary), ...Object.keys(prSummary)]);
109+
allFiles.delete('total');
110+
const workspacePath = process.env.GITHUB_WORKSPACE ? process.env.GITHUB_WORKSPACE + '/' : '';
111+
112+
let changedRows = [];
113+
let newRows = [];
114+
115+
for (const file of allFiles) {
116+
const baseFile = baseSummary[file];
117+
const prFile = prSummary[file];
118+
119+
if (!prFile) continue;
120+
121+
const oldPct = getPct(baseFile, metric);
122+
const newPct = getPct(prFile, metric);
123+
124+
const relativeFilePath = file.replace(workspacePath, '');
125+
const linkedPath = `[${relativeFilePath}](${fileUrl(relativeFilePath)})`;
126+
127+
if (!baseFile && prFile) {
128+
newRows.push(`| ${linkedPath} (**new**) | — | ${newPct.toFixed(2)}% | — |\n`);
129+
} else if (oldPct !== newPct) {
130+
changedRows.push(`| ${linkedPath} | ${oldPct.toFixed(2)}% | ${newPct.toFixed(2)}% | ${formatDiff(oldPct, newPct)} |\n`);
131+
}
132+
}
133+
134+
if (changedRows.length === 0 && newRows.length === 0) {
135+
markdown += `\n_No coverage changes._\n`;
136+
} else {
137+
markdown += `| | Base | PR | Delta |\n`;
138+
markdown += `| :--- | :---: | :---: | :---: |\n`;
139+
140+
if (changedRows.length > 0) {
141+
markdown += changedRows.sort().join('');
142+
}
143+
if (newRows.length > 0) {
144+
markdown += newRows.sort().join('');
145+
}
146+
147+
const oldTotalPct = getPct(baseSummary.total, metric);
148+
const newTotalPct = getPct(prSummary.total, metric);
149+
if (oldTotalPct !== newTotalPct) {
150+
markdown += `| **Total** | ${oldTotalPct.toFixed(2)}% | ${newTotalPct.toFixed(2)}% | ${formatDiff(oldTotalPct, newTotalPct)} |\n`;
151+
}
152+
}
153+
154+
markdown += `\n\n_Generated by [coverage-comment.yml](https://github.com/${owner}/${repo}/actions/workflows/coverage-comment.yml)_`;
155+
156+
const prNumber = fs.readFileSync('./PR_NUMBER', 'utf8').trim();
157+
158+
if (!prNumber) {
159+
console.log("No PR number found.");
160+
return;
161+
}
162+
163+
const comments = await github.rest.issues.listComments({
164+
owner,
165+
repo,
166+
issue_number: prNumber,
167+
});
168+
169+
const existingComment = comments.data.find(c =>
170+
c.body.includes('Generated by [coverage-comment.yml]') &&
171+
c.user.type === 'Bot'
172+
);
173+
174+
if (existingComment) {
175+
await github.rest.issues.updateComment({
176+
owner,
177+
repo,
178+
comment_id: existingComment.id,
179+
body: markdown
180+
});
181+
} else {
182+
await github.rest.issues.createComment({
183+
owner,
184+
repo,
185+
issue_number: prNumber,
186+
body: markdown
187+
});
188+
}

.github/workflows/unit-tests.yml

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
---
22
name: Run Unit Tests
33
on:
4-
pull_request:
4+
push:
55
branches: [main, 1.0-dev]
6+
pull_request:
67
permissions:
78
contents: read
9+
810
jobs:
911
test:
1012
name: Test with Python ${{ matrix.python-version }}
@@ -54,9 +56,72 @@ jobs:
5456
echo "$HOME/.cargo/bin" >> $GITHUB_PATH
5557
- name: Install Buf
5658
uses: bufbuild/buf-setup-action@v1
59+
60+
# Coverage comparison for PRs (only on Python 3.13 to avoid duplicate work)
61+
- name: Checkout Base Branch
62+
if: github.event_name == 'pull_request' && matrix.python-version == '3.13'
63+
uses: actions/checkout@v4
64+
with:
65+
ref: ${{ github.event.pull_request.base.ref || 'main' }}
66+
clean: true
67+
5768
- name: Install dependencies
5869
run: uv sync --locked
59-
- name: Run tests and check coverage
70+
71+
- name: Run coverage (Base)
72+
if: github.event_name == 'pull_request' && matrix.python-version == '3.13'
73+
run: |
74+
uv run pytest --cov=a2a --cov-report=json --cov-report=html:coverage
75+
mv coverage.json /tmp/coverage-base.json
76+
77+
- name: Checkout PR Branch (Restore)
78+
if: github.event_name == 'pull_request' && matrix.python-version == '3.13'
79+
uses: actions/checkout@v4
80+
with:
81+
clean: true
82+
83+
- name: Install dependencies (PR)
84+
if: github.event_name == 'pull_request' && matrix.python-version == '3.13'
85+
run: uv sync --locked && uv build
86+
87+
- name: Run coverage (PR)
88+
if: github.event_name == 'pull_request' && matrix.python-version == '3.13'
89+
run: |
90+
uv run pytest --cov=a2a --cov-report=json --cov-report=html:coverage --cov-report=term --cov-fail-under=88
91+
mv coverage.json coverage-pr.json
92+
cp /tmp/coverage-base.json coverage-base.json
93+
94+
- name: Save Metadata
95+
if: github.event_name == 'pull_request' && matrix.python-version == '3.13'
96+
run: |
97+
echo ${{ github.event.number }} > ./PR_NUMBER
98+
echo ${{ github.event.pull_request.base.ref || 'main' }} > ./BASE_BRANCH
99+
100+
- name: Upload Coverage Artifacts
101+
uses: actions/upload-artifact@v4
102+
if: github.event_name == 'pull_request' && matrix.python-version == '3.13'
103+
with:
104+
name: coverage-data
105+
path: |
106+
coverage-base.json
107+
coverage-pr.json
108+
coverage/
109+
PR_NUMBER
110+
BASE_BRANCH
111+
retention-days: 1
112+
113+
# Run standard tests (for matrix items that didn't run coverage PR)
114+
- name: Run tests (Standard)
115+
if: matrix.python-version != '3.13' || github.event_name != 'pull_request'
60116
run: uv run pytest --cov=a2a --cov-report term --cov-fail-under=88
117+
118+
- name: Upload Artifact (base)
119+
uses: actions/upload-artifact@v4
120+
if: github.event_name != 'pull_request' && matrix.python-version == '3.13'
121+
with:
122+
name: coverage-report
123+
path: coverage
124+
retention-days: 14
125+
61126
- name: Show coverage summary in log
62127
run: uv run coverage report

.pre-commit-config.yaml

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,27 +31,6 @@ repos:
3131
# ===============================================
3232
# Python Hooks
3333
# ===============================================
34-
# no_implicit_optional for ensuring explicit Optional types
35-
- repo: https://github.com/hauntsaninja/no_implicit_optional
36-
rev: '1.4'
37-
hooks:
38-
- id: no_implicit_optional
39-
args: [--use-union-or]
40-
41-
# Pyupgrade for upgrading Python syntax to newer versions
42-
- repo: https://github.com/asottile/pyupgrade
43-
rev: v3.20.0
44-
hooks:
45-
- id: pyupgrade
46-
args: [--py310-plus] # Target Python 3.10+ syntax, matching project's target
47-
48-
# Autoflake for removing unused imports and variables
49-
- repo: https://github.com/pycqa/autoflake
50-
rev: v2.3.1
51-
hooks:
52-
- id: autoflake
53-
args: [--in-place, --remove-all-unused-imports]
54-
5534
# Ruff for linting and formatting
5635
- repo: https://github.com/astral-sh/ruff-pre-commit
5736
rev: v0.12.0

pyproject.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,6 @@ dev = [
123123
"types-protobuf",
124124
"types-requests",
125125
"pre-commit",
126-
"pyupgrade",
127-
"autoflake",
128-
"no_implicit_optional",
129126
"trio",
130127
"uvicorn>=0.35.0",
131128
"pytest-timeout>=2.4.0",

scripts/format.sh

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,6 @@ if [ -n "$CHANGED_FILES" ]; then
7575
echo "Files to be formatted:"
7676
echo "$CHANGED_FILES"
7777

78-
echo "Running autoflake..."
79-
run_formatter "$CHANGED_FILES" autoflake -i -r --remove-all-unused-imports
8078
echo "Running ruff check (fix-only)..."
8179
run_formatter "$CHANGED_FILES" ruff check --fix-only $RUFF_UNSAFE_FIXES_FLAG
8280
echo "Running ruff format..."

src/a2a/compat/v0_3/rest_adapter.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import functools
2+
import json
23
import logging
34

45
from collections.abc import AsyncIterable, AsyncIterator, Awaitable, Callable
@@ -103,9 +104,9 @@ async def _handle_streaming_request(
103104

104105
async def event_generator(
105106
stream: AsyncIterable[Any],
106-
) -> AsyncIterator[dict[str, dict[str, Any]]]:
107+
) -> AsyncIterator[str]:
107108
async for item in stream:
108-
yield {'data': item}
109+
yield json.dumps(item)
109110

110111
return EventSourceResponse(
111112
event_generator(method(request, call_context))

src/a2a/compat/v0_3/rest_handler.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import logging
22

3-
from collections.abc import AsyncIterable, AsyncIterator
3+
from collections.abc import AsyncIterator
44
from typing import TYPE_CHECKING, Any
55

6-
from google.protobuf.json_format import MessageToDict, MessageToJson, Parse
6+
from google.protobuf.json_format import MessageToDict, Parse
77

88

99
if TYPE_CHECKING:
@@ -86,7 +86,7 @@ async def on_message_send_stream(
8686
self,
8787
request: Request,
8888
context: ServerCallContext,
89-
) -> AsyncIterator[str]:
89+
) -> AsyncIterator[dict[str, Any]]:
9090
"""Handles the 'message/stream' REST method.
9191
9292
Args:
@@ -108,7 +108,7 @@ async def on_message_send_stream(
108108
v03_pb_resp = proto_utils.ToProto.stream_response(
109109
v03_stream_resp.result
110110
)
111-
yield MessageToJson(v03_pb_resp)
111+
yield MessageToDict(v03_pb_resp)
112112

113113
async def on_cancel_task(
114114
self,
@@ -142,7 +142,7 @@ async def on_subscribe_to_task(
142142
self,
143143
request: Request,
144144
context: ServerCallContext,
145-
) -> AsyncIterable[str]:
145+
) -> AsyncIterator[dict[str, Any]]:
146146
"""Handles the 'tasks/{id}:subscribe' REST method.
147147
148148
Args:
@@ -164,7 +164,7 @@ async def on_subscribe_to_task(
164164
v03_pb_resp = proto_utils.ToProto.stream_response(
165165
v03_stream_resp.result
166166
)
167-
yield MessageToJson(v03_pb_resp)
167+
yield MessageToDict(v03_pb_resp)
168168

169169
async def get_push_notification(
170170
self,

0 commit comments

Comments
 (0)