You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The PR adds browser-based ws-wasm-agent coverage tests, a coverage WebSocket server, ChromeDriver mise tooling, and a wasm-agent-cov task that merges wasm and native coverage into lcov.info.
Changes
WebAssembly coverage integration
Layer / File(s)
Summary
ChromeDriver tooling and policy .mise/config.toml, config/conftest/policy/mise/mise.rego
Adds a pinned OS-scoped ChromeDriver tool, updates the Chrome browser task to resolve it with mise which, and allowlists the tool.
Coverage WebSocket server lifecycle services/ws-test-server/Cargo.toml, services/ws-test-server/src/lib.rs, services/ws-test-server/src/bin/cov-server.rs, .codacy.yaml
Adds the coverage server binary, readiness-marker shutdown flow, explicit-port startup support, its filesystem dependency, and a Codacy exclusion for the trusted marker path.
Adds the coverage feature and browser tests for client configuration, connection behavior, queuing, callbacks, DOM helpers, JavaScript reflection, live messaging, reconnection, and tracing.
Coverage collection and aggregation .mise/config.coverage.toml, .github/workflows/coverage.yaml
Adds the wasm-agent-cov task and invokes it from CI to process wasm and native profiles into lcov.info.
sequenceDiagram
participant Workflow as GitHub Actions coverage
participant Mise as wasm-agent-cov
participant Server as cov-server
participant Tests as wasm-bindgen tests
participant LLVM as LLVM coverage tools
participant Lcov as lcov.info
Workflow->>Mise: Run coverage task with CHROMEDRIVER
Mise->>Server: Start instrumented server
Mise->>Tests: Run browser tests
Tests-->>Mise: Emit wasm coverage profiles
Mise->>LLVM: Merge profiles and export lcov
Server-->>LLVM: Provide native coverage
LLVM->>Lcov: Append filtered coverage
We reviewed changes in 37f9429...7662d6a on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR establishes a robust coverage infrastructure for the WASM agent using headless Chrome and a specialized test server (cov-server). All core acceptance criteria and recommended test scenarios have been implemented. However, the PR description is currently empty, providing no context for these infrastructure changes.
While the code is considered 'Up to Standards' by Codacy, the new integration test file services/ws-wasm-agent/tests/client.rs has high complexity (delta +20) and contains several reliability issues. Specifically, the suite lacks proper environment isolation due to DOM pollution and non-idempotent tracing initialization, which can cause intermittent failures or panics in CI. These issues should be resolved to ensure a stable testing pipeline.
About this PR
The PR description is empty. Please provide a high-level summary of the infrastructure changes, specifically how the headless Chrome environment and the cov-server interact to produce coverage results.
Test suggestions
Verify agent state transitions from disconnected to connecting and finally connected.
Verify offline message queuing behavior and the dropping of oldest messages when capacity (1000) is exceeded.
Verify reconnection logic and exponential backoff when a connection attempt fails.
Verify DOM helper functions for setting and appending text to textarea elements.
Verify JS reflection helpers for reading numeric, boolean, and nested object fields.
Verify full end-to-end connectivity and message dispatch against the instrumented cov-server backend.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Appending elements to the document body without removing them causes DOM pollution. Tests should be isolated; consider removing the elements at the end of the test.
Try running the following prompt in your coding agent:
Refactor the textarea_helpers_set_and_append test in services/ws-wasm-agent/tests/client.rs to ensure that all elements appended to the document body are removed at the end of the test, ideally using an RAII guard or explicit remove_child calls.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Suggestion: Tracing initialization can only happen once per instance. To prevent the test suite from panicking if this test is executed multiple times or in parallel, the underlying initialization should be made idempotent.
Try running the following prompt in your coding agent:
Modify the init_tracing function in et_ws_wasm_agent to use std::sync::Once so it can be safely called multiple times without panicking.
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Nitpick: The SERVER_URL constant is defined for centralizing the test server address, but this location still uses a hardcoded string. Use the constant to ensure consistency.
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Nitpick: The documentation states this file exercises logic 'without a server', but connects_flushes_queue_and_sends depends on a live backend. Update the header to clarify that while most tests are offline, some integration coverage requires the test server.
This might be a simple fix:
Suggested change
//! Server-independent coverage of the WASM agent's client logic.
The reason will be displayed to describe this comment to others. Learn more.
`storage.clear()` erases unrelated `localStorage` keys used by other tests
storage.clear() mutates shared browser state for the whole origin. When tests execute in different orders, this can break unrelated cases and create flaky CI failures that are hard to reproduce.
Remove broad clearing and avoid asserting globally persisted state here. Keep the test focused on WsClientConfig setters and disconnected state.
Add per-platform checksums for the ChromeDriver archives. Versioned URLs alone don’t pin the archive contents; use per-platform checksum fields or a mise.lock entry.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.mise/config.toml around lines 242 - 260, Add integrity verification for the
archives configured under tools."http:chromedriver" by supplying the correct
per-platform checksum for each linux-x64, macos-arm64, and macos-x64 URL. Keep
the existing version, platform selection, URLs, and binary configuration
unchanged; use checksum fields or the repository’s established mise.lock
mechanism.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.mise/config.coverage.toml:
- Line 208: Update the EXIT trap in the coverage configuration to remove the
readiness marker and reap the instrumented server during shutdown, ensuring this
cleanup runs even when setup fails before main returns. Retain kill only as a
bounded fallback after the normal shutdown/reaping path, while preserving the
existing failure cleanup behavior.
In `@services/ws-wasm-agent/tests/client.rs`:
- Line 8: Replace the crate-level target_arch gate in the client test target so
non-wasm32 invocations fail loudly instead of silently running zero tests.
Direct the failure message to the wasm32 mise task, while preserving normal
browser test execution on wasm32.
---
Nitpick comments:
In @.mise/config.toml:
- Around line 242-260: Add integrity verification for the archives configured
under tools."http:chromedriver" by supplying the correct per-platform checksum
for each linux-x64, macos-arm64, and macos-x64 URL. Keep the existing version,
platform selection, URLs, and binary configuration unchanged; use checksum
fields or the repository’s established mise.lock mechanism.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 910bfb83-0689-4ab1-9cf8-3d136d59be9f
📥 Commits
Reviewing files that changed from the base of the PR and between 37f9429 and 7662d6a.
The EXIT trap directly kills the instrumented server, so failures before Line 224 prevent main() from returning and
can discard its native profile. Have the trap remove the readiness marker and reap the process, with kill only as a
bounded fallback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.mise/config.coverage.toml at line 208, Update the EXIT trap in the coverage
configuration to remove the readiness marker and reap the instrumented server
during shutdown, ensuring this cleanup runs even when setup fails before main
returns. Retain kill only as a bounded fallback after the normal
shutdown/reaping path, while preserving the existing failure cleanup behavior.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not silently disable the entire test target outside wasm32.
This makes an incorrectly invoked test command succeed with zero browser tests. Obtain an explicit exception or replace
the silent gate with a fail-loud guard directing callers to the wasm32 mise task.
As per coding guidelines, tests must never be platform-disabled without explicit user approval, and missing tools must
fail loudly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ws-wasm-agent/tests/client.rs` at line 8, Replace the crate-level
target_arch gate in the client test target so non-wasm32 invocations fail loudly
instead of silently running zero tests. Direct the failure message to the wasm32
mise task, while preserving normal browser test execution on wasm32.
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
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.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary by CodeRabbit
New Features
Bug Fixes
Refactor