Skip to content
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
2 changes: 1 addition & 1 deletion crates/blitz-control-protocol/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "blitz-control-protocol"
description = "The Blitz agent-control and diagnostics surface: one vocabulary, one core, two transports"
version = "0.5.3"
version = "0.5.4"
edition.workspace = true
rust-version.workspace = true
license.workspace = true
Expand Down
11 changes: 11 additions & 0 deletions crates/blitz-control-protocol/src/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1898,6 +1898,7 @@ pub(crate) fn keyboard_modifiers(modifiers: ControlModifiers) -> KeyboardModifie
output.set(KeyboardModifiers::CONTROL, modifiers.control);
output.set(KeyboardModifiers::ALT, modifiers.alt);
output.set(KeyboardModifiers::META, modifiers.meta);
output.set(KeyboardModifiers::SUPER, modifiers.meta);
output
}

Expand Down Expand Up @@ -2151,6 +2152,16 @@ mod tests {
use super::*;
use blitz_dom::{Document, DocumentConfig};

#[test]
fn meta_also_sets_the_super_modifier() {
let modifiers = keyboard_modifiers(ControlModifiers {
meta: true,
..ControlModifiers::default()
});
assert!(modifiers.contains(KeyboardModifiers::META));
assert!(modifiers.contains(KeyboardModifiers::SUPER));
}

/// A control whose whole size is its label, next to one with padding.
///
/// `font-size: 0` stands in for the host this exists for: a Linux CI
Expand Down
2 changes: 1 addition & 1 deletion crates/ps-qa/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "ps-qa"
description = "Drive a running Blitz app through its MCP control socket and assert what the renderer did"
version = "0.7.5"
version = "0.7.6"
edition = "2024"
rust-version = "1.88"
license = "MIT OR Apache-2.0"
Expand Down
5 changes: 5 additions & 0 deletions crates/ps-qa/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ name, `click --id` selects the intended row without coordinates.

For repeatable drag outcomes, declare
`pointer_drag: Some((from: "button:Drag handle", dx: 60.0, dy: 30.0, steps: 3))`
can instead use a painted destination:
`pointer_drag: Some((from: "button:Drag handle", to: Some("#target"), steps: 12))`.
ps-qa resolves both centers at run time, so a data-derived target can move
without copying its coordinates into the check file. Use either `to` or
`dx`/`dy` for one drag.
on a check, then assert the resulting state through `subject` and `expect`.
Adding `cancel: true` sends pointercancel instead of pointerup. This drives
pointer capture and movement; the older `drag` diagnostic directly scrolls a
Expand Down
8 changes: 8 additions & 0 deletions crates/ps-qa/src/interaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,20 @@ pub(crate) async fn pointer_drag(
dy: f64,
steps: u32,
cancel: bool,
to: Option<&str>,
) -> Result<()> {
if !dx.is_finite() || !dy.is_finite() || steps == 0 || steps > 240 {
bail!("pointer drag needs finite offsets and 1..=240 steps");
}
let destination = if let Some(to) = to {
let (_, bounds) = locate_control(client, to, &[]).await?;
Some((bounds[0] + bounds[2] / 2.0, bounds[1] + bounds[3] / 2.0))
} else {
None
};
let (_, bounds) = locate_control(client, want, &[]).await?;
let start = (bounds[0] + bounds[2] / 2.0, bounds[1] + bounds[3] / 2.0);
let (dx, dy) = destination.map_or((dx, dy), |end| (end.0 - start.0, end.1 - start.1));
let request = |phase, x, y| {
AgentControlRequest::Act(AgentAction::Input(InputCommand::Pointer {
phase,
Expand Down
95 changes: 75 additions & 20 deletions crates/ps-qa/src/qa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,14 @@ pub enum Expect {
#[serde(deny_unknown_fields)]
pub struct PointerDrag {
pub from: String,
/// Optional painted destination; its center determines the drag offset.
/// This lets a QA page expose data-derived coordinates without copying them
/// into a static check manifest.
#[serde(default)]
pub to: Option<String>,
#[serde(default)]
pub dx: f64,
#[serde(default)]
pub dy: f64,
pub steps: u32,
#[serde(default)]
Expand All @@ -376,7 +383,15 @@ pub struct Check {
pub group: String,
/// What this proves, in the words you would use to report it.
pub what: String,
/// Press this first, to reach the surface the check is about.
/// Load a URL in the standing document before opening a surface.
///
/// Relative URLs resolve against the document that is up. Use this when a
/// route must load as a fresh document, such as reloading a signed-in
/// account page that client-side navigation cannot reach.
#[serde(default)]
pub navigate: Option<String>,
/// Activate this after [`navigate`](Self::navigate), when set, to reach the
/// surface the check is about.
///
/// Checks run in sequence against one instance and start wherever the app
/// opens, so anything not on that first surface is unreachable without a
Expand Down Expand Up @@ -545,18 +560,16 @@ pub struct Check {
/// rendered action still has to pass the one-second budget.
#[serde(default)]
pub settle_after_ms: u64,
/// Deadline for this check's [`open`](Self::open) step.
///
/// Navigation is not the interaction contract. `open` may be a route change
/// that fetches before it can paint, and a live network round trip lands on
/// either side of the 900ms every other step gets. The failure that
/// produces is also the wrong sentence: `could not open "Crates"` reads as
/// a missing tab rather than as a deadline, and the reader goes looking for
/// a control that is there.
///
/// So a route that is known to fetch declares what it costs here, and every
/// other navigation in the suite keeps the strict default rather than being
/// weakened to cover the slow one.
/// Deadline for this check's [`navigate`](Self::navigate) and
/// [`open`](Self::open) steps.
///
/// Navigation is not the interaction contract. `navigate` loads a document,
/// and `open` can activate a route that fetches before it paints. A live
/// network round trip can exceed the 900ms default, so the route can declare
/// its arrival budget without weakening every other check.
///
/// Use this only for a route known to fetch. Without it, the failure reads
/// as a missing control rather than as a deadline.
#[serde(default)]
pub open_timeout_ms: u64,
/// Deadline for this check's rendered outcome.
Expand Down Expand Up @@ -727,21 +740,22 @@ pub fn checks(dir: Option<&std::path::Path>) -> Result<Vec<Check>, String> {
*/
if index == 0
&& check.open.is_none()
&& check.navigate.is_none()
&& let Some((previous_file, opener)) = &established
{
return Err(format!(
concat!(
"{}: check {:?} is the first in its file and declares no `open`, so it ",
"would run on {:?}, which {} navigated to. Declare the surface this ",
"file starts on."
"{}: check {:?} is the first in its file and declares neither `navigate` nor ",
"`open`, so it would run on {:?}, which {} navigated to. Declare how this ",
"file establishes its starting surface."
),
file.display(),
check.id,
opener,
previous_file.display(),
));
}
if let Some(opener) = check.open.as_deref() {
if let Some(opener) = check.open.as_deref().or(check.navigate.as_deref()) {
established = Some((file.to_path_buf(), opener.to_owned()));
}
}
Expand Down Expand Up @@ -856,6 +870,17 @@ fn validate_check(
));
}

if let Some(drag) = &check.pointer_drag
&& let Some(to) = &drag.to
&& (to.is_empty() || drag.dx != 0.0 || drag.dy != 0.0)
{
return Err(format!(
"{}: check {:?} must use either a non-empty pointer drag destination or dx/dy offsets",
file.display(),
check.id,
));
}

/*
* A weakening that does nothing must not look like it did something.
*
Expand Down Expand Up @@ -1711,10 +1736,12 @@ fn action_description(check: &Check) -> String {
}
if let Some(drag) = &check.pointer_drag {
let description = format!(
"drag {:?} by {},{} in {} steps{}",
"drag {:?} {} in {} steps{}",
drag.from,
drag.dx,
drag.dy,
drag.to.as_ref().map_or_else(
|| format!("by {},{}", drag.dx, drag.dy),
|to| format!("to {to:?}"),
),
drag.steps,
if drag.cancel {
" and cancel"
Expand Down Expand Up @@ -1774,6 +1801,15 @@ mod tests {
ron::from_str(&ron).expect("check parses")
}

#[test]
fn navigate_is_optional_and_read_from_check_files() {
assert_eq!(parse("").navigate, None);
assert_eq!(
parse("navigate:Some(\"/account\"),").navigate.as_deref(),
Some("/account")
);
}

fn painted_node(id: u64, name: &str, width: f64, height: f64) -> SemanticNode {
SemanticNode {
dom_id: None,
Expand Down Expand Up @@ -2057,6 +2093,24 @@ mod tests {
);
}

#[test]
fn a_drag_can_name_its_painted_destination() {
let mut check =
parse("pointer_drag:Some((from:\"button:Piece\",to:Some(\"#target\"),steps:12)),");
check.click = None;
let drag = check.pointer_drag.as_ref().expect("drag is configured");
assert_eq!(drag.to.as_deref(), Some("#target"));
assert_eq!(drag.dx, 0.0);
assert_eq!(drag.dy, 0.0);
assert!(action_description(&check).contains("drag \"button:Piece\" to \"#target\""));

let mut invalid = check.clone();
invalid.pointer_drag.as_mut().unwrap().dx = 20.0;
let error = validate_check(&invalid, Path::new("drag.ron"), &mut HashMap::new())
.expect_err("a named destination and displacement are ambiguous");
assert!(error.contains("either a non-empty pointer drag destination or dx/dy"));
}

#[test]
fn checks_can_prepare_with_a_key_without_changing_the_measured_action() {
let check = parse("prepare:Some(\"Menu\"),prepare_key:Some(\"ArrowDown\"),");
Expand Down Expand Up @@ -2263,6 +2317,7 @@ mod tests {
group: "settings".into(),
what: "the slider moves".into(),
open: None,
navigate: None,
prepare: None,
prepare_unless: None,
prepare_press: false,
Expand Down
38 changes: 35 additions & 3 deletions crates/ps-qa/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,7 @@ async fn run_qa(
* on another surface.
*/
/*
* Navigation is best-effort: already being on the surface is success.
* `open` is best-effort: already being on the surface is success.
*
* Checks run in sequence, so a later one often inherits exactly the
* screen it would have navigated to, and the control it navigates *by*
Expand All @@ -1171,7 +1171,36 @@ async fn run_qa(
let mut open_error = None;
let mut pixel_outcome: Option<std::result::Result<(), String>> = None;
let open_budget = declared_open_timeout(check);
if let Some(want) = check.open.as_deref() {
if let Some(url) = check.navigate.as_deref() {
let navigated = client
.agent(&AgentControlRequest::Navigate {
url: url.to_owned(),
})
.await;
if let Err(error) = navigated {
open_error = Some(format!("could not navigate to {url:?}: {error}"));
} else {
let target = check
.open
.as_deref()
.or_else(|| check.hover.as_ref().map(qa::Hover::target))
.or(check.prepare.as_deref())
.or(check.click.as_deref())
.or(check.type_into.as_deref())
.or(check.key_on.as_deref())
.unwrap_or(&check.subject);
if !wait_for_arrival(client, None, target, open_budget).await? {
open_error = Some(format!(
"could not navigate to {url:?}: the arrival target {target:?} did not paint within \
{}ms. Raise open_timeout_ms if this route fetches.",
open_budget.as_millis()
));
}
}
}
if open_error.is_none()
&& let Some(want) = check.open.as_deref()
{
/*
* A permanent surface marker can answer "already there". A
* document marker cannot: every document renders the same
Expand Down Expand Up @@ -1987,6 +2016,7 @@ async fn run_qa(
drag.dy,
drag.steps,
drag.cancel,
drag.to.as_deref(),
)
.await
{
Expand Down Expand Up @@ -6045,7 +6075,7 @@ pub async fn run() -> Result<()> {
steps,
cancel,
} => {
pointer_drag(&mut client, &name, dx, dy, steps, cancel).await?;
pointer_drag(&mut client, &name, dx, dy, steps, cancel, None).await?;
}
// Direct container scrolling, retained for existing diagnostic commands.
cli::Command::Drag { name, dy, steps } => {
Expand Down Expand Up @@ -6945,6 +6975,7 @@ mod tests {
group: "coverage".into(),
what: "a rendered outcome".into(),
open: None,
navigate: None,
prepare: None,
prepare_unless: None,
prepare_press: false,
Expand Down Expand Up @@ -7381,6 +7412,7 @@ mod tests {
drag.expect = Expect::ValueChanges;
drag.pointer_drag = Some(PointerDrag {
from: "slider:Hue".into(),
to: None,
dx: 20.0,
dy: 0.0,
steps: 2,
Expand Down
Loading