Skip to content

test(scripts): dispatch a script of our own instead of hunting for someone else's - #2243

Merged
giokur merged 1 commit into
mainfrom
test/script-history-self-sufficient
Sep 18, 2026
Merged

giokur merged 1 commit into
mainfrom
test/script-history-self-sufficient

Conversation

@giokur

@giokur giokur commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

My previous fix for this was wrong

Read a script's execution history and facets has aborted on every nightly on record. In #2220 I
moved it into the post-mingo phase on the theory that it simply ran before anything dispatched a
script.

The 2026-09-18 nightly disproved that cleanly. The case ran in post-mingo, on a freshly registered
tenant, after mingo had created and run scripts successfully at 02:26:17 and 02:29:22 — and it
aborted anyway.

The actual cause

scriptWithHistory() walks the script list and wants one that still exists and has executions:

for (Script candidate : ScriptApi.listScripts()) {
    if (ScriptApi.getExecutionFilters(candidate.getId()).getFilteredCount() > 0) return candidate;
}
return null;

MingoDeviceTest.@AfterEach deletes every script it creates (scriptsToCleandeleteScript). So
the scripts mingo dispatches are gone before the phase ends, and no candidate ever survives. Ordering
was never the problem, and no amount of reordering could have fixed it.

Both causes — "runs before its data exists" and "its data is cleaned up by whoever made it" — produce
the identical abort message. That is how the wrong reading stuck for two days.

The fix

Stop depending on found state. The case now creates a script, dispatches it to an ONLINE Windows
device, polls until the execution lands, and then runs the existing history and facet assertions
against its own script. The class already owned and cleaned up one script for the schedule half; this
adds a second and deletes it the same way.

That needed a dispatch binding the library did not have, so this also adds ScriptApi.runScript over
the product's existing runScript(input: RunScriptInput!): DispatchResponse!. The unused
RunScriptRequest DTO is left alone — different, older shape, nothing calls it.

The move to post-mingo was still the enabling step

Dispatching needs an enrolled ONLINE machine, and one only exists after the device and assistant
phases. In functional this fix would not be possible at all. The class javadoc and the tag comment
both asserted the old, wrong reason and are corrected to say this.

Still skips in one case, on purpose

If the tenant has no ONLINE Windows device the case still skips. That is an environment precondition
rather than missing found state, and asserting a page of zero rows against zero facets would pass
hollowly.

Module compiles.

🤖 Generated with Claude Code

…meone else's

`Read a script's execution history and facets` has aborted on every nightly on
record. My previous fix (#2220) moved it into the `post-mingo` phase on the
theory that it simply ran too early. That was wrong, and the 2026-09-18 nightly
proved it: the case ran in `post-mingo`, on a fresh tenant, after mingo had
created and run scripts successfully at 02:26:17 and 02:29:22 — and aborted
anyway.

The real cause is that `scriptWithHistory()` needs a script that still exists AND
has executions, and `MingoDeviceTest.@AfterEach` deletes every script it creates.
No candidate ever survives the phase, so no amount of reordering helps. Both
causes emit the same abort message, which is how the wrong one stuck.

Stop depending on found state: create a script, dispatch it to an ONLINE Windows
device, poll until the execution lands, then run the existing history and facet
assertions against it. The class already owned and cleaned up a script for the
schedule half; this adds a second and deletes it the same way.

Needs a dispatch binding, which the library did not have — `ScriptApi.runScript`
over the product's `runScript(input: RunScriptInput!): DispatchResponse!`. The
unused `RunScriptRequest` DTO is left alone; it is a different, older shape.

The move to `post-mingo` was not wasted: dispatching needs an enrolled ONLINE
machine and one only exists after the device and assistant phases. The class
javadoc and the tag comment both said the opposite of what is true and are
corrected.

Still skips, legitimately, when the tenant has no ONLINE Windows device — that is
an environment precondition, not missing found state.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

🦩 Flamingo Code Review

No findings on the current head.

Mode: advisory


Need another pass? Commits pushed after this review are not reviewed automatically.

  • Review the new commits — the commits added since this review
  • Review the whole diff again — ignoring what was already reviewed

Prefer typing? Comment @flamingo-review, or @flamingo-review full. To review every push on this pull request, add the flamingo-review-always label.

React 👍/👎 on inline comments to teach the reviewer.

Started 2026-09-18 06:10 UTC · updated 2026-09-18 06:11 UTC · workflow run

@giokur
giokur enabled auto-merge (squash) September 18, 2026 06:56
@giokur
giokur merged commit a1e25e0 into main Sep 18, 2026
13 of 14 checks passed
@giokur
giokur deleted the test/script-history-self-sufficient branch September 18, 2026 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants