mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-02 19:17:10 +00:00
fix(chrome): anchor the transition deadline when the dom goes quiet
it was anchored at script start, so a page that churned past the window reached the check already expired and returned mid cross-fade. the driver now publishes the idle timeout it needs, since the caller's 1s could never spend the 800ms window.
This commit is contained in:
1 parent
f738a590f0
commit
973ca16a41
2 files changed
+149
-12
No files matched your search
@@ -557,12 +557,35 @@ const domQuietPeriod = 150 * time.Millisecond
|
|||||||
// shows two *Screen ids at rest costs this much per step and no more.
|
// shows two *Screen ids at rest costs this much per step and no more.
|
||||||
const transitionSettlePeriod = 800 * time.Millisecond
|
const transitionSettlePeriod = 800 * time.Millisecond
|
||||||
|
|
||||||
|
// settleReturnMargin is what WaitForIdle holds back from the caller's timeout,
|
||||||
|
// so returning late by our own doing surfaces as a settled page rather than a
|
||||||
|
// context cancellation.
|
||||||
|
const settleReturnMargin = 100 * time.Millisecond
|
||||||
|
|
||||||
|
// settleScanMargin covers the in-page work the two waits do not themselves
|
||||||
|
// account for: liveScreens() walks the document and every shadow root on each
|
||||||
|
// 16 ms poll, and the whole script costs one CDP round trip.
|
||||||
|
const settleScanMargin = 250 * time.Millisecond
|
||||||
|
|
||||||
|
// MinIdleTimeout is the shortest timeout WaitForIdle can be handed and still
|
||||||
|
// spend the waits it is built from: the DOM quiet period, the route-transition
|
||||||
|
// window that only opens once that quiet period has elapsed, and the second
|
||||||
|
// quiet period the transition's own closing mutation starts. A caller that
|
||||||
|
// passes less caps the settle below its own budget, and the step then samples a
|
||||||
|
// page that is still mid-transition - which is the exact failure the transition
|
||||||
|
// wait exists to prevent. internal/runner raises a shorter caller timeout to
|
||||||
|
// this value.
|
||||||
|
func (d *Driver) MinIdleTimeout() time.Duration {
|
||||||
|
return 2*domQuietPeriod + transitionSettlePeriod +
|
||||||
|
settleScanMargin + settleReturnMargin
|
||||||
|
}
|
||||||
|
|
||||||
func (d *Driver) WaitForIdle(ctx context.Context, timeout time.Duration) error {
|
func (d *Driver) WaitForIdle(ctx context.Context, timeout time.Duration) error {
|
||||||
runCtx, cancel := d.runCtx(ctx)
|
runCtx, cancel := d.runCtx(ctx)
|
||||||
defer cancel()
|
defer cancel()
|
||||||
// Leave the caller's deadline some room: returning late by our own doing
|
// Leave the caller's deadline some room: returning late by our own doing
|
||||||
// would surface as a context cancellation instead of a settled page.
|
// would surface as a context cancellation instead of a settled page.
|
||||||
budget := max(timeout-100*time.Millisecond, domQuietPeriod)
|
budget := max(timeout-settleReturnMargin, domQuietPeriod)
|
||||||
script := fmt.Sprintf(settleScript,
|
script := fmt.Sprintf(settleScript,
|
||||||
domQuietPeriod.Milliseconds(),
|
domQuietPeriod.Milliseconds(),
|
||||||
budget.Milliseconds(),
|
budget.Milliseconds(),
|
||||||
@@ -598,11 +621,18 @@ const liveScreensFunction = `
|
|||||||
// itself gives up after %d ms. Shadow roots get their own observer: a canvas
|
// itself gives up after %d ms. Shadow roots get their own observer: a canvas
|
||||||
// app keeps its whole accessibility tree inside one, and mutations there do not
|
// app keeps its whole accessibility tree inside one, and mutations there do not
|
||||||
// reach an observer on the document.
|
// reach an observer on the document.
|
||||||
|
//
|
||||||
|
// The transition window opens when the quiet period ends, not when the script
|
||||||
|
// starts. Anchored at the start it is already spent by the time the check can
|
||||||
|
// first run on any page that keeps mutating for longer than the window, so the
|
||||||
|
// wait resolves immediately with both routes still live - the mid-transition
|
||||||
|
// return this whole wait exists to prevent. Each mutation reopens it, and the
|
||||||
|
// budget above bounds the total either way.
|
||||||
const settleScript = `
|
const settleScript = `
|
||||||
new Promise(resolve => {
|
new Promise(resolve => {
|
||||||
const quietMillis = %d, budgetMillis = %d, transitionMillis = %d;
|
const quietMillis = %d, budgetMillis = %d, transitionMillis = %d;
|
||||||
const observers = [];
|
const observers = [];
|
||||||
const transitionDeadline = Date.now() + transitionMillis;
|
let transitionDeadline = 0;
|
||||||
let timer = null;
|
let timer = null;
|
||||||
const finish = () => {
|
const finish = () => {
|
||||||
clearTimeout(timer);
|
clearTimeout(timer);
|
||||||
@@ -611,6 +641,7 @@ new Promise(resolve => {
|
|||||||
};
|
};
|
||||||
` + liveScreensFunction + `
|
` + liveScreensFunction + `
|
||||||
const quiet = () => {
|
const quiet = () => {
|
||||||
|
if (transitionDeadline === 0) transitionDeadline = Date.now() + transitionMillis;
|
||||||
if (liveScreens() > 1 && Date.now() < transitionDeadline) {
|
if (liveScreens() > 1 && Date.now() < transitionDeadline) {
|
||||||
timer = setTimeout(quiet, 16);
|
timer = setTimeout(quiet, 16);
|
||||||
return;
|
return;
|
||||||
@@ -619,6 +650,7 @@ new Promise(resolve => {
|
|||||||
};
|
};
|
||||||
const restart = () => {
|
const restart = () => {
|
||||||
clearTimeout(timer);
|
clearTimeout(timer);
|
||||||
|
transitionDeadline = 0;
|
||||||
timer = setTimeout(quiet, quietMillis);
|
timer = setTimeout(quiet, quietMillis);
|
||||||
};
|
};
|
||||||
const watch = (root) => {
|
const watch = (root) => {
|
||||||
@@ -770,15 +802,19 @@ func (d *Driver) EvaluateExtractors(ctx context.Context) (map[int]json.RawMessag
|
|||||||
// action was actually applied. Without this call every web state.lastAction is
|
// action was actually applied. Without this call every web state.lastAction is
|
||||||
// null, so a property gated on what the last action did is vacuously true and
|
// null, so a property gated on what the last action did is vacuously true and
|
||||||
// reports a green run while checking nothing.
|
// reports a green run while checking nothing.
|
||||||
|
//
|
||||||
|
// The call is deliberately unguarded. A `setter && setter(...)` form evaluates
|
||||||
|
// to undefined on a page whose runtime does not define the setter, and chromedp
|
||||||
|
// reports that as success, so "the page cannot accept lastAction" would be
|
||||||
|
// indistinguishable from "installed". That page is reachable: a run resolving
|
||||||
|
// its web runtime from an older published @sanderling/spec would silently no-op
|
||||||
|
// every step. Unguarded, the missing global throws and the run fails loudly.
|
||||||
func (d *Driver) SetLastAction(ctx context.Context, encoded json.RawMessage) error {
|
func (d *Driver) SetLastAction(ctx context.Context, encoded json.RawMessage) error {
|
||||||
payload := strings.TrimSpace(string(encoded))
|
payload := strings.TrimSpace(string(encoded))
|
||||||
if payload == "" {
|
if payload == "" {
|
||||||
payload = "null"
|
payload = "null"
|
||||||
}
|
}
|
||||||
script := fmt.Sprintf(
|
script := fmt.Sprintf(`window.__sanderlingSetLastAction__(%s)`, payload)
|
||||||
`window.__sanderlingSetLastAction__ && window.__sanderlingSetLastAction__(%s)`,
|
|
||||||
payload,
|
|
||||||
)
|
|
||||||
runCtx, cancel := d.runCtx(ctx)
|
runCtx, cancel := d.runCtx(ctx)
|
||||||
defer cancel()
|
defer cancel()
|
||||||
if err := chromedp.Run(runCtx, chromedp.Evaluate(script, nil)); err != nil {
|
if err := chromedp.Run(runCtx, chromedp.Evaluate(script, nil)); err != nil {
|
||||||
@@ -789,16 +825,25 @@ func (d *Driver) SetLastAction(ctx context.Context, encoded json.RawMessage) err
|
|||||||
|
|
||||||
// extractorScript resolves the extractor table once the page is not mid route
|
// extractorScript resolves the extractor table once the page is not mid route
|
||||||
// transition, giving up on that wait after %d ms.
|
// transition, giving up on that wait after %d ms.
|
||||||
|
//
|
||||||
|
// A missing table rejects rather than reporting {}, for the same reason
|
||||||
|
// SetLastAction no longer guards its call: an empty override map is what a
|
||||||
|
// spec with no extractors returns, so the guarded form made "this page has no
|
||||||
|
// sanderling runtime" read as a normal step whose properties then ran on
|
||||||
|
// goja's dump-derived values instead of the page's.
|
||||||
const extractorScript = `
|
const extractorScript = `
|
||||||
new Promise(resolve => {
|
new Promise((resolve, reject) => {
|
||||||
const deadline = Date.now() + %d;` + liveScreensFunction + `
|
const deadline = Date.now() + %d;` + liveScreensFunction + `
|
||||||
const read = () => {
|
const read = () => {
|
||||||
if (liveScreens() > 1 && Date.now() < deadline) {
|
if (liveScreens() > 1 && Date.now() < deadline) {
|
||||||
setTimeout(read, 16);
|
setTimeout(read, 16);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
resolve(JSON.stringify(
|
if (typeof window.__sanderlingExtractors__ !== "function") {
|
||||||
window.__sanderlingExtractors__ ? window.__sanderlingExtractors__() : {}));
|
reject(new Error("__sanderlingExtractors__ is not installed in the page"));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
resolve(JSON.stringify(window.__sanderlingExtractors__()));
|
||||||
};
|
};
|
||||||
read();
|
read();
|
||||||
})`
|
})`
|
||||||
|
|||||||
@@ -672,17 +672,31 @@ func TestWaitForIdle_ReturnsOnABusyPage(t *testing.T) {
|
|||||||
// app is leaving, which on the folio wasm build recorded a submit that had
|
// app is leaving, which on the folio wasm build recorded a submit that had
|
||||||
// landed on Home as still being on the transaction screen: the route gate of an
|
// landed on Home as still being on the transaction screen: the route gate of an
|
||||||
// action-gated property then skipped the very step the action landed on.
|
// action-gated property then skipped the very step the action landed on.
|
||||||
|
//
|
||||||
|
// The page keeps mutating for 300 ms after the route splice, and the settle
|
||||||
|
// runs on the timeout production hands it (MinIdleTimeout). Both details are
|
||||||
|
// load-bearing. A quiet page reaches the transition check immediately, so it
|
||||||
|
// passes whether the transition window is anchored at the script start or at
|
||||||
|
// the end of the quiet period; churn is what pushes the check past a
|
||||||
|
// start-anchored deadline, which then finishes at once with two live screens.
|
||||||
|
// And a caller timeout below MinIdleTimeout cuts the whole settle off before
|
||||||
|
// the transition window can be spent, which is the same bug from the other end.
|
||||||
func TestWaitForIdle_WaitsOutARouteTransition(t *testing.T) {
|
func TestWaitForIdle_WaitsOutARouteTransition(t *testing.T) {
|
||||||
const page = `<body><div id="app"></div><script>
|
const page = `<body><div id="app"></div><script>
|
||||||
const root = document.getElementById("app").attachShadow({mode: "open"});
|
const root = document.getElementById("app").attachShadow({mode: "open"});
|
||||||
root.innerHTML = '<button id="go" style="width:200px;height:80px">go</button>' +
|
root.innerHTML = '<button id="go" style="width:200px;height:80px">go</button>' +
|
||||||
'<div id="LedgerScreen">ledger</div>';
|
'<div id="LedgerScreen">ledger</div><div id="spinner">0</div>';
|
||||||
root.getElementById("go").addEventListener("click", function () {
|
root.getElementById("go").addEventListener("click", function () {
|
||||||
const incoming = document.createElement("div");
|
const incoming = document.createElement("div");
|
||||||
incoming.id = "HomeScreen";
|
incoming.id = "HomeScreen";
|
||||||
incoming.textContent = "home";
|
incoming.textContent = "home";
|
||||||
root.appendChild(incoming);
|
root.appendChild(incoming);
|
||||||
setTimeout(function () { root.getElementById("LedgerScreen").remove(); }, 400);
|
let frame = 0;
|
||||||
|
const churn = setInterval(function () {
|
||||||
|
root.getElementById("spinner").textContent = String(++frame);
|
||||||
|
}, 30);
|
||||||
|
setTimeout(function () { clearInterval(churn); }, 300);
|
||||||
|
setTimeout(function () { root.getElementById("LedgerScreen").remove(); }, 1000);
|
||||||
});
|
});
|
||||||
</script></body>`
|
</script></body>`
|
||||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||||
@@ -701,7 +715,7 @@ func TestWaitForIdle_WaitsOutARouteTransition(t *testing.T) {
|
|||||||
if err := d.Tap(ctx, 40, 40); err != nil {
|
if err := d.Tap(ctx, 40, 40); err != nil {
|
||||||
t.Fatalf("Tap: %v", err)
|
t.Fatalf("Tap: %v", err)
|
||||||
}
|
}
|
||||||
if err := d.WaitForIdle(ctx, 2*time.Second); err != nil {
|
if err := d.WaitForIdle(ctx, d.MinIdleTimeout()); err != nil {
|
||||||
t.Fatalf("WaitForIdle: %v", err)
|
t.Fatalf("WaitForIdle: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -793,3 +807,81 @@ func TestEvaluateExtractors_WaitsOutARouteTransition(t *testing.T) {
|
|||||||
"mid-transition, so the spec sees the route the app is leaving", got)
|
"mid-transition, so the spec sees the route the app is leaving", got)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestSetLastAction_ReportsAPageThatCannotTakeIt covers the install the whole
|
||||||
|
// web path's action-gated properties hang off. A page without the setter is
|
||||||
|
// reachable: internal/testrun resolves the web runtime from
|
||||||
|
// node_modules/@sanderling/spec when no sibling checkout is present, and an
|
||||||
|
// older published runtime does not define it. Guarded as
|
||||||
|
// `setter && setter(...)`, that page returns undefined and chromedp reports
|
||||||
|
// success, so every step silently no-ops and every property gated on the last
|
||||||
|
// action goes vacuously true - a green run that checked nothing.
|
||||||
|
func TestSetLastAction_ReportsAPageThatCannotTakeIt(t *testing.T) {
|
||||||
|
const withSetter = `<body><script>
|
||||||
|
window.__lastActionSeen = null;
|
||||||
|
window.__sanderlingSetLastAction__ = function (value) { window.__lastActionSeen = value; };
|
||||||
|
</script></body>`
|
||||||
|
const withoutSetter = `<body><div id="app">no sanderling runtime here</div></body>`
|
||||||
|
pages := map[string]string{"/with": withSetter, "/without": withoutSetter}
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.Header().Set("Content-Type", "text/html")
|
||||||
|
_, _ = w.Write([]byte(pages[r.URL.Path]))
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
d := New()
|
||||||
|
defer d.Terminate(context.Background())
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
|
if err := d.Launch(ctx, server.URL+"/with", false, nil); err != nil {
|
||||||
|
t.Fatalf("Launch: %v", err)
|
||||||
|
}
|
||||||
|
action := json.RawMessage(`{"kind":"Tap","on":"id:TxnSubmit"}`)
|
||||||
|
if err := d.SetLastAction(ctx, action); err != nil {
|
||||||
|
t.Fatalf("SetLastAction on a page that defines the setter: %v", err)
|
||||||
|
}
|
||||||
|
var seen map[string]string
|
||||||
|
if err := chromedp.Run(d.tabCtx,
|
||||||
|
chromedp.Evaluate(`window.__lastActionSeen`, &seen)); err != nil {
|
||||||
|
t.Fatalf("read installed action: %v", err)
|
||||||
|
}
|
||||||
|
if seen["on"] != "id:TxnSubmit" {
|
||||||
|
t.Errorf("the page received %v, want the action the runner applied", seen)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := d.Launch(ctx, server.URL+"/without", false, nil); err != nil {
|
||||||
|
t.Fatalf("Launch: %v", err)
|
||||||
|
}
|
||||||
|
if err := d.SetLastAction(ctx, action); err == nil {
|
||||||
|
t.Error("SetLastAction reported success on a page with no setter; " +
|
||||||
|
"a runtime that cannot take lastAction is indistinguishable from one that did")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestEvaluateExtractors_ReportsAMissingTable is the same failure on the other
|
||||||
|
// sampler. An empty override map is what a spec with no extractors returns, so
|
||||||
|
// treating a missing table as {} makes "this page has no sanderling runtime"
|
||||||
|
// read as an ordinary step - and the verifier then judges the run on goja's
|
||||||
|
// dump-derived values while believing they came from the page.
|
||||||
|
func TestEvaluateExtractors_ReportsAMissingTable(t *testing.T) {
|
||||||
|
const page = `<body><div id="app">no sanderling runtime here</div></body>`
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||||
|
w.Header().Set("Content-Type", "text/html")
|
||||||
|
_, _ = w.Write([]byte(page))
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
d := New()
|
||||||
|
defer d.Terminate(context.Background())
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||||
|
defer cancel()
|
||||||
|
if err := d.Launch(ctx, server.URL, false, nil); err != nil {
|
||||||
|
t.Fatalf("Launch: %v", err)
|
||||||
|
}
|
||||||
|
values, err := d.EvaluateExtractors(ctx)
|
||||||
|
if err == nil {
|
||||||
|
t.Errorf("EvaluateExtractors returned %v and no error on a page with no "+
|
||||||
|
"extractor table; a page that cannot be read must not read as empty", values)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user