Skip to content

Commit f4ccec6

Browse files
committed
fix(ci): close silent-skip gap in integration split (review #7293)
Reviewer correctly flagged that 22 integration test functions carry the integration marker but no p0/p1/p2 priority marker, so the three shard expressions silently excluded them (browser real-page journeys, session isolation, token rotation, WS auth, ACP MCP scope, Windows native host repair). Fix (both suggested approaches combined): 1. Assign priority markers to all 22 unclassified tests (19 -> p1, 3 -> p2), so the three priority shards cover the full suite again. 2. Add a fourth 'fallback' shard with expression 'integration and not (p0 or p1 or p2)': any future unclassified test still RUNS (never silently skipped) instead of vanishing. 3. Add a fail-closed guard: the ubuntu fallback shard fails the run if it collects anything, forcing new tests to get a priority marker. 4. Tolerate empty fallback: pytest exit 5 (no tests collected) is expected there; coverage upload uses if-no-files-found: ignore and the combine step picks up whichever shard files exist. AST audit after fix: 0 unclassified integration tests. 署名:秦琼·CIOps@QPQAT
1 parent 5f0183b commit f4ccec6

9 files changed

Lines changed: 93 additions & 16 deletions

.github/workflows/tests.yml

Lines changed: 69 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ jobs:
276276
matrix:
277277
python-version: ["3.11", "3.13"]
278278
os: [ubuntu-latest]
279-
shard: [p0, p1, p2]
279+
shard: [p0, p1, p2, fallback]
280280
include:
281281
- os: macos-latest
282282
python-version: "3.11"
@@ -287,6 +287,9 @@ jobs:
287287
- os: macos-latest
288288
python-version: "3.11"
289289
shard: p2
290+
- os: macos-latest
291+
python-version: "3.11"
292+
shard: fallback
290293
- os: windows-latest
291294
python-version: "3.11"
292295
shard: p0
@@ -296,6 +299,9 @@ jobs:
296299
- os: windows-latest
297300
python-version: "3.11"
298301
shard: p2
302+
- os: windows-latest
303+
python-version: "3.11"
304+
shard: fallback
299305

300306
steps:
301307
- uses: actions/checkout@v4
@@ -381,16 +387,42 @@ jobs:
381387
if [ -n "${DISPATCH_MARKER}" ]; then
382388
EXPR="${DISPATCH_MARKER}"
383389
else
384-
# Split by shard for parallel execution
390+
# Split by shard for parallel execution. The fallback shard
391+
# catches any integration test that lacks a p0/p1/p2 marker
392+
# so it can never be silently skipped.
385393
case "${{ matrix.shard }}" in
386394
p0) EXPR="integration and p0" ;;
387395
p1) EXPR="integration and p1" ;;
388396
p2) EXPR="integration and p2" ;;
397+
fallback) EXPR="integration and not (p0 or p1 or p2)" ;;
389398
esac
390399
fi
391400
echo "expr=$EXPR" >> "$GITHUB_OUTPUT"
392401
echo "Selected marker expression: $EXPR"
393402
403+
- name: Fail on unclassified integration tests
404+
if: |
405+
steps.check-integrated.outputs.has_tests == 'true' &&
406+
matrix.shard == 'fallback' &&
407+
matrix.os == 'ubuntu-latest' &&
408+
matrix.python-version == '3.11'
409+
shell: bash
410+
run: |
411+
# Guard: every integration test must carry a priority marker.
412+
# If the fallback shard collects anything, a new unclassified
413+
# test slipped in -- fail loudly instead of silently running
414+
# it outside the three priority shards.
415+
UNCLASSIFIED=$(python -m pytest tests/integration --collect-only -q \
416+
-m "integration and not (p0 or p1 or p2)" 2>/dev/null \
417+
| grep -c "::" || true)
418+
if [ "${UNCLASSIFIED}" -gt 0 ]; then
419+
echo "::error::${UNCLASSIFIED} integration test(s) lack a p0/p1/p2 priority marker. Assign one so the test joins a priority shard."
420+
python -m pytest tests/integration --collect-only -q \
421+
-m "integration and not (p0 or p1 or p2)" 2>/dev/null | grep "::" || true
422+
exit 1
423+
fi
424+
echo "No unclassified integration tests."
425+
394426
- name: Run integrated tests
395427
if: steps.check-integrated.outputs.has_tests == 'true'
396428
shell: bash
@@ -412,16 +444,30 @@ jobs:
412444
pytest tests/integration -v --no-cov \
413445
-n auto --dist=loadscope --timeout=300 \
414446
-m "${{ steps.marker.outputs.expr }}"
415-
cp .integration_coverage/integration_subproc \
416-
.coverage.integration.${{ matrix.shard }}
417-
# `coverage xml` honours fail_under and exits 2 when below;
418-
# tolerate that — the combined value is what matters.
419-
coverage xml --data-file=.coverage.integration.${{ matrix.shard }} \
420-
-o coverage.integration.${{ matrix.shard }}.xml || [ "$?" -eq 2 ]
447+
PYTEST_RC=$?
448+
# exit 5 = no tests collected: expected for the fallback
449+
# shard when every integration test carries a priority
450+
# marker. Any other nonzero code still fails.
451+
if [ "$PYTEST_RC" -ne 0 ] && [ "$PYTEST_RC" -ne 5 ]; then
452+
exit "$PYTEST_RC"
453+
fi
454+
if [ -f .integration_coverage/integration_subproc ]; then
455+
cp .integration_coverage/integration_subproc \
456+
.coverage.integration.${{ matrix.shard }}
457+
# `coverage xml` honours fail_under and exits 2 when below;
458+
# tolerate that — the combined value is what matters.
459+
coverage xml --data-file=.coverage.integration.${{ matrix.shard }} \
460+
-o coverage.integration.${{ matrix.shard }}.xml || [ "$?" -eq 2 ]
461+
fi
421462
else
422463
pytest tests/integration -v \
423464
-n auto --dist=loadscope --timeout=300 \
424-
-m "${{ steps.marker.outputs.expr }}"
465+
-m "${{ steps.marker.outputs.expr }}" || {
466+
RC=$?
467+
# exit 5 = no tests collected: expected for the
468+
# fallback shard when nothing is unclassified.
469+
[ "$RC" -eq 5 ] || exit "$RC"
470+
}
425471
fi
426472
427473
- name: Upload integration coverage data
@@ -437,6 +483,10 @@ jobs:
437483
coverage.integration.${{ matrix.shard }}.xml
438484
retention-days: 1
439485
include-hidden-files: true
486+
# The fallback shard produces no data file when every
487+
# integration test carries a priority marker; a missing
488+
# artifact there is expected, not an error.
489+
if-no-files-found: ignore
440490

441491
coverage-report:
442492
name: Coverage Report
@@ -480,13 +530,16 @@ jobs:
480530
- name: Combine all coverage data
481531
shell: bash
482532
run: |
483-
# First, combine the three integration shards into one
484-
coverage combine --data-file=.coverage.integration \
485-
.coverage.integration.p0 \
486-
.coverage.integration.p1 \
487-
.coverage.integration.p2
488-
coverage xml --data-file=.coverage.integration \
489-
-o coverage.integration.xml || [ "$?" -eq 2 ]
533+
# First, combine the integration shards into one. The fallback
534+
# shard only produces a data file when it actually ran tests
535+
# (i.e. some integration test lacked a priority marker), so
536+
# include whichever shard files exist.
537+
SHARDS=$(ls .coverage.integration.* 2>/dev/null || true)
538+
if [ -n "$SHARDS" ]; then
539+
coverage combine --data-file=.coverage.integration $SHARDS
540+
coverage xml --data-file=.coverage.integration \
541+
-o coverage.integration.xml || [ "$?" -eq 2 ]
542+
fi
490543
491544
# Then, combine all three tiers: unit, contract, and integration.
492545
coverage combine \

tests/integration/browser/test_browser_tool_e2e.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
from collections.abc import AsyncGenerator
55

6+
import pytest
67
import pytest_asyncio
78
from agentscope.message import ToolResultState
89

@@ -26,6 +27,7 @@ async def reset_kernel() -> AsyncGenerator[None, None]:
2627
kernel._MANAGER = None # pylint: disable=protected-access
2728

2829

30+
@pytest.mark.p1
2931
async def test_browser_tool_drives_a_real_page(fixture_url: str) -> None:
3032
code = (
3133
"browser = await Browser.connect(identity='guest')\n"

tests/integration/browser/test_e2e_isolation.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ def request(session_id: str, code: str) -> ExecRequest:
2121
)
2222

2323

24+
@pytest.mark.p1
2425
async def test_two_incognito_sessions_are_isolated(fixture_url: str) -> None:
2526
plane = SubprocessPlane()
2627
first_code = (
@@ -55,6 +56,7 @@ async def test_two_incognito_sessions_are_isolated(fixture_url: str) -> None:
5556
await link.close_all()
5657

5758

59+
@pytest.mark.p1
5860
async def test_same_session_id_is_isolated_by_workspace(
5961
fixture_url: str,
6062
) -> None:
@@ -118,6 +120,7 @@ async def test_same_session_id_is_isolated_by_workspace(
118120
"contexts",
119121
[("profile", "incognito"), ("incognito", "profile")],
120122
)
123+
@pytest.mark.p1
121124
async def test_profile_and_incognito_have_independent_process_cells(
122125
contexts: tuple[str, str],
123126
tmp_path,

tests/integration/browser/test_execution_lifecycle_e2e.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import os
1010
import time
1111

12+
import pytest
1213
from fastapi import FastAPI
1314

1415
from qwenpaw.app._app import _start_browser_runtime, _stop_browser_runtime
@@ -30,6 +31,7 @@ def _request(
3031
)
3132

3233

34+
@pytest.mark.p1
3335
async def test_worker_is_reused_then_reclaimed() -> None:
3436
plane = SubprocessPlane()
3537
request = _request("reuse", "session", "import os\nreturn os.getpid()")
@@ -46,6 +48,7 @@ async def test_worker_is_reused_then_reclaimed() -> None:
4648
await plane.discard_all_workers()
4749

4850

51+
@pytest.mark.p1
4952
async def test_sibling_sessions_run_without_serializing() -> None:
5053
"""Sibling sessions must run in parallel, not queue behind one lock.
5154
@@ -103,6 +106,7 @@ async def test_sibling_sessions_run_without_serializing() -> None:
103106
await plane.discard_all_workers()
104107

105108

109+
@pytest.mark.p1
106110
async def test_timeout_reclaims_only_the_affected_worker() -> None:
107111
plane = SubprocessPlane(exec_timeout_seconds=5.0)
108112
runtime = KernelRuntime(plane=plane)
@@ -128,6 +132,7 @@ async def test_timeout_reclaims_only_the_affected_worker() -> None:
128132
await plane.discard_all_workers()
129133

130134

135+
@pytest.mark.p1
131136
async def test_runtime_shutdown_reclaims_real_workers() -> None:
132137
plane = SubprocessPlane()
133138
runtime = KernelRuntime(plane=plane)

tests/integration/browser/test_playwright_provider_e2e.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ def _locator_spec(method: str, *args: str, **kwargs: str) -> list[dict]:
1919
]
2020

2121

22+
@pytest.mark.p1
2223
async def test_real_provider_isolates_sessions_and_profile(
2324
fixture_url: str,
2425
tmp_path,
@@ -62,6 +63,7 @@ async def test_real_provider_isolates_sessions_and_profile(
6263
await link.close_all()
6364

6465

66+
@pytest.mark.p1
6567
async def test_real_provider_observes_locates_and_acts(
6668
fixture_url: str,
6769
) -> None:
@@ -106,6 +108,7 @@ async def test_real_provider_observes_locates_and_acts(
106108
await link.close_all()
107109

108110

111+
@pytest.mark.p1
109112
async def test_real_provider_rejects_unsupported_context_and_method() -> None:
110113
link = PlaywrightControlLink()
111114
try:

tests/integration/browser/test_token_rotation.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,15 @@ def bridge_config(
3636
return config_path
3737

3838

39+
@pytest.mark.p1
3940
def test_rotation_recovers(bridge_config: Path) -> None:
4041
_write_config(bridge_config, "token-a")
4142
assert ws_handler._expected_token() == "token-a"
4243
_write_config(bridge_config, "token-b") # plugin repair(reset) rotates
4344
assert ws_handler._expected_token() == "token-b"
4445

4546

47+
@pytest.mark.p1
4648
def test_missing_file_falls_back_to_cache(bridge_config: Path) -> None:
4749
_write_config(bridge_config, "token-a")
4850
assert ws_handler._expected_token() == "token-a"
@@ -51,6 +53,7 @@ def test_missing_file_falls_back_to_cache(bridge_config: Path) -> None:
5153
assert not bridge_config.exists() # fallback must not rewrite or rotate
5254

5355

56+
@pytest.mark.p1
5457
def test_bootstrap_generates_once(bridge_config: Path) -> None:
5558
token = ws_handler._expected_token()
5659
assert token
@@ -72,6 +75,7 @@ def websocket_client(
7275
return TestClient(app)
7376

7477

78+
@pytest.mark.p1
7579
def test_ws_handshake_after_rotation(
7680
websocket_client: TestClient,
7781
bridge_config: Path,

tests/integration/browser/test_ws_bridge_auth.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ def websocket_client(monkeypatch: pytest.MonkeyPatch) -> TestClient:
2626
return TestClient(app)
2727

2828

29+
@pytest.mark.p1
2930
def test_nm_bridge_denies_wrong_token(websocket_client: TestClient) -> None:
3031
with pytest.raises(WebSocketDenialResponse) as denied:
3132
with websocket_client.websocket_connect(
@@ -36,6 +37,7 @@ def test_nm_bridge_denies_wrong_token(websocket_client: TestClient) -> None:
3637
assert denied.value.status_code == 401
3738

3839

40+
@pytest.mark.p1
3941
def test_nm_bridge_accepts_correct_token(websocket_client: TestClient) -> None:
4042
with websocket_client.websocket_connect("/ws/chrome?token=expected-token"):
4143
pass

tests/integration/test_acp_mcp_driver_flow.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ async def request_approval(
3838
self.contexts.append(context)
3939

4040

41+
@pytest.mark.p1
4142
@pytest.mark.asyncio
4243
async def test_acp_mcp_card_discovers_approves_and_invokes_tool(
4344
tmp_path: Path,
@@ -108,6 +109,7 @@ async def test_acp_mcp_card_discovers_approves_and_invokes_tool(
108109
assert await manager.card_store.list_paths() == []
109110

110111

112+
@pytest.mark.p1
111113
@pytest.mark.asyncio
112114
async def test_acp_mcp_tool_cannot_be_invoked_without_session_scope(
113115
tmp_path: Path,

tests/integration/test_chrome_native_host_install.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ def test_successful_install_records_a_passing_probe(
100100
assert result["installed"] is True
101101

102102

103+
@pytest.mark.p2
103104
def test_non_reset_repair_preserves_existing_bridge_token(
104105
isolated_home: Path,
105106
monkeypatch: pytest.MonkeyPatch,
@@ -140,13 +141,15 @@ def test_non_reset_repair_preserves_existing_bridge_token(
140141
),
141142
],
142143
)
144+
@pytest.mark.p2
143145
def test_windows_batch_path_literal_normalizes_and_escapes(
144146
source: str,
145147
expected: str,
146148
) -> None:
147149
assert extension_setup._windows_batch_path_literal(source) == expected
148150

149151

152+
@pytest.mark.p2
150153
def test_windows_launcher_uses_cmd_safe_path_literals(
151154
isolated_home: Path,
152155
monkeypatch: pytest.MonkeyPatch,

0 commit comments

Comments
 (0)