Control over where automation and e2e runs (BL-16804) - #8285
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile has paused reviews on this repository — it used its 100 free open-source review credits for this billing period. Reviews resume automatically on September 24. To continue before then, an organization admin can keep reviews running past the free credits — those bill as normal usage.
84dd838 to
0a07db4
Compare
Every run of the src/BloomE2E suite opened a Bloom window on the developer's desktop, which makes the suite unusable while working. A new --headless startup flag puts the shell and the splash screen far to the left of every monitor and keeps them out of the task bar, and Bloom does not save those bounds as the window placement. The window goes off-screen rather than minimized or hidden because WebView2 stops painting a minimized window, which makes every screenshot blank; off-screen it paints exactly as it would in front of a person, so rendering and keyboard input behave the same. Windows reports a window that far out as occluded, and Chromium then stops rendering it, so a headless run also turns off CalculateNativeWinOcclusion and the backgrounding of occluded windows. Shell sets ShowInTaskbar in its constructor rather than in Shell_Load, where the rest of the headless placement happens. Shell_Load runs during Show(), when the form already has a window handle, and Windows Forms answers an assignment to ShowInTaskbar there by recreating that handle and every child handle under it, including the WebView2 host. The Edit tab was left with a browser that no longer answered editView/jumpToPage, so every e2e test that moves between pages hung until goToPage gave up. The e2e fixture passes the flag by default. BLOOM_E2E_HEADED=1 shows the window, and Playwright's --debug implies it, because stepping through a test whose window you cannot see is pointless. Split out of BL-16799 so that it can be reviewed on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0a07db4 to
4c4e2bd
Compare
|
[Claude Opus 5 (1M context) from Hatton's machine during preflight] Consulted Devin on 2026-09-02, up to commit
Devin's two informational notes confirm the design and are not mirrored here. Also for the record, at commit |
…ifference Master now says the espagnol / espanol difference in the Text Languages test is machine-dependent, not a flake: LibPalaso needs a findable native ICU library, and Bloom ships icu.net without icuuc.dll. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BLOOM_AUTOMATION_MONITOR becomes the only control, and every window an automation run opens obeys it, the splash screen included. It takes a 1-based monitor number, the word "headless" for off every monitor, or anything else, which leaves Bloom to place its windows as it always does. This replaces three controls that each did part of the job: the --headless startup flag, the BLOOM_E2E_HEADED variable in the Playwright fixture, and a silent fallback to the primary monitor when the variable named a monitor the machine did not have. A value Bloom cannot use now gives a visible window, which is the outcome that tells the developer the variable did not take effect. The new class AutomationWindowPlacement holds the whole decision. Its Parse method takes the raw value and the monitor count as arguments, so the 21 new unit tests need neither the environment variable nor a machine with a particular set of monitors. The off-screen position leaves four window widths of clearance, not a fixed 1000 pixels. Bloom computes the position in its own coordinate space, and on a monitor scaled to 160% a window it believed was 1587 pixels wide came out 2560 pixels wide, which left 27 pixels of the old cushion. Windows scales a monitor by at most 400%, so four widths clears the leftmost monitor whatever the scale factors are. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Before this branch the Playwright fixture passed --headless on every launch, so the nightly ran off-screen. Removing that flag left nothing in its place, so the nightly would have started opening a window without anybody choosing that. The Run BloomE2E tests step now sets BLOOM_AUTOMATION_MONITOR=headless. Devin raised the consequence. It suggested making off-screen the default in the fixture instead, which is declined: a developer's own run showing a window is the point of the change. The workflow is the one caller that cannot ask for what it wants at the moment it runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BLOOM_AUTOMATION_MONITOR took a 1-based index into Screen.AllScreens, which is the order of the display drivers. That order means nothing a developer can see: on a three-monitor machine it was 1 (right), 2 (left), 3 (primary, centre), while Windows Settings printed 1 (primary, centre), 2 (right), 3 (left). So a developer who read a number off Windows Settings got a different monitor, and nothing said so. Windows Settings' own numbers cannot be reproduced. QueryDisplayConfig with QDC_ONLY_ACTIVE_PATHS returns the paths in exactly the AllScreens order, the \.\DISPLAY<n> device names do not match either, and Microsoft documents no rule. So count left to right instead: monitor 1 is the leftmost. That follows the arrangement picture in Windows Settings, which a developer can read, even though it is not the numbers printed on it. Also accept "0" as a synonym for "headless". It reads as "no monitor" beside the numbers that name one, and it is quicker to type. Bloom now writes the whole mapping to its log on every automation start, so a developer can see which monitor a number means rather than guess. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin again, up to It raised one non-severe Bug for this commit, "Normal fallback ignores saved placement". That is its own thread on Also green at this commit: the Greptile is out of free credits until 2026-09-24, by its own comment, so it has not reviewed this branch. |
An off-screen window asked for the primary monitor's working-area size, but Windows gives a window the scale factor of the monitor nearest to it. Out to the left, the nearest monitor is the leftmost one, whose scale factor is very likely not the primary's. On a machine with a 150% primary and a 100% monitor beside it, a window meant to match the primary's 3840x2100 came out 3840x2100 REAL pixels, taller than any monitor on the machine, and the page inside it laid out at a viewport height of 1990 CSS pixels that no user could ever have. format-gear-positioning.spec.ts failed on exactly that: it asserts the format gear sits in the lower half of the view, and the gear landed at 846 of 1990. The file passes on monitor 2 (150%) and on monitor 3 (100%) and failed only off-screen, on the same build, in back-to-back runs, which is what isolated it. So the window now goes straight down from the primary monitor, in line with its left edge. The primary stays the nearest monitor, so an off-screen window paints at the size a visible one would. The clearance rule is unchanged in substance: four window heights below the lowest monitor, plus 1000 pixels, clamped to 32000, for the same reason the old rule left four widths. This is the second bug from one cause. The first ate all but 27 pixels of a 1000-pixel cushion. Both passed every unit test, because a unit test compares numbers inside one process's own coordinate space, so AUTOMATION-DEBT.md now carries an entry asking for the ability to run the suite at a chosen resolution and scale factor. The full suite off-screen is back to 23 passed, 2 failed, 10 did not run, the two failures being the pre-existing pair on BL-16807. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The suite sizes its window to whatever monitor it lands on, so it proves the layout only at the size of a developer's screen. That hides a class of bugs that users on inexpensive machines meet and we never do. The change itself is small, so the entry states the design: 1024x586 (the working area of a 1024x768 screen) for every run, BLOOM_AUTOMATION_WINDOW_SIZE to ask for something else, a 400x300 floor, and the same size whatever BLOOM_AUTOMATION_MONITOR says. The reason it is debt rather than a change is what it costs. A full run at 1024x586 gave 16 passed, 6 failed and 13 not run, against 23 passed and 2 failed at monitor size; the entry names the four tests the small window breaks and the triage each one needs. The developer chose to record the plan rather than carry a red suite, so the code that was written for it is not in the history and the entry says how to rebuild it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Devin's review of the previous commit is right on the geometry. The comment claimed that keeping the off-screen window directly under the primary keeps the primary the nearest monitor, and Windows applies the nearest monitor's scale factor. That claim holds only while no monitor sits below the primary in the same band of x. Stack one monitor under another at a different scale and the lower one is nearest, so the window comes out the wrong size again. Nobody on the team has such a layout, and getting it right for every layout means asking Windows for the nearest monitor's scale factor and scaling the requested size by the ratio, which needs the per-monitor DPI calls the automation-debt entry is about. So this records the limit in both places rather than overstating what the code does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| // monitor. | ||
| const int farDownWindowsAllows = 32000; | ||
| const int largestScaleFactorWindowsAllows = 4; | ||
| var lowestY = Screen.AllScreens.Max(screen => screen.Bounds.Bottom); |
There was a problem hiding this comment.
[Claude Opus 5 from Hatton's machine during preflight]
Devin (non-severe Bug): "Lower monitors restore DPI mismatch." Devin says that placing the
off-screen window below the lowest monitor does not, in general, keep the primary monitor the
nearest one. If a monitor sits below the primary and overlaps it horizontally, that lower monitor
is nearest, Windows applies its scale factor, and the window comes out the wrong size again.
Devin is right on the geometry, and the comment here claimed more than the code delivers. The
primary is nearest only while no monitor sits below the primary in the same band of x.
What this means in practice:
- On every layout anyone on the team has, the monitors sit side by side, so the primary is nearest
and the placement is correct. The two real DPI failures this PR fixed both came from going
sideways, which is wrong on ordinary layouts, not from going down. - Fixing it for every layout means asking Windows for the scale factor of whichever monitor ends
up nearest and scaling the requested size by the ratio. That needs the per-monitor DPI calls
Microsoft does not document, which is a different piece of work.
So rather than overstate what the code does, 6f2a42d244 says plainly where it stops being true,
in the comment and in src/BloomE2E/AUTOMATION-DEBT.md under "No way to run the suite at a chosen
monitor resolution and scale factor", where the fix has a home.
This one is for the developer to decide, so the thread stays open: take the recorded limit, or
spend the per-monitor DPI work now.
There was a problem hiding this comment.
Decided to live with this. This business of picking the DPI of a neighboring monitor is completely arbitrary. Anyhow, in the future we will pay more attention to specifying what DPI and scale you want to run at.
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin again, up to Two Bugs it still lists are from earlier commits in the branch:
The other two Bugs and the one Investigate flag are ones Devin now marks resolved. Two CI ( |
…wise The class comment said a developer who leaves the variable set in their shell still gets an ordinary, visible Bloom when they start one themselves. That is false: ./go.sh passes --automation (scripts/watchBloomExe.mjs), so a Bloom started to work in obeys the variable too. The behaviour is right and stays. It is what the variable is for: the developer sets it once and every Bloom an agent starts stays off the monitor they are working on. Only the comment was wrong. It now also records the one consequence worth knowing, because a reader who meets it without warning loses an afternoon: "headless" hides a ./go.sh Bloom the developer started themselves, and a hidden Bloom looks exactly like one that failed to start. The log line DescribeChoice writes is what settles that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hatton
changed the title
1 participant
BL-16804 — https://issues.bloomlibrary.org/youtrack/issue/BL-16804
Split out of BL-16799 so the window-placement change can be reviewed on its own.
The problem
Every run of the
src/BloomE2Esuite opened a Bloom window on the developer's desktop, which makes the suite unusable while working. Three separate controls decided where that window went, and none of them was discoverable: an environment variableBLOOM_AUTOMATION_MONITORthat only the main window read, a silent fallback to the primary monitor, and aBLOOM_E2E_HEADEDvariable in the Playwright fixture.The fix
BLOOM_AUTOMATION_MONITORbecomes the one control, and every window an automation run opens obeys it, the splash screen included:headless, or0— every window opens far outside every monitor, so a run can go on while you work.The variable applies only under
--automation, so a Bloom you start yourself is unaffected however it is set. The nightly workflow now asks forheadlessexplicitly.Four things that are not obvious, each explained in a code comment where it matters:
CalculateNativeWinOcclusiondisabled and--disable-backgrounding-occluded-windows.ShowInTaskbaris set in the constructor. Assigning it to a form that is already showing makes WinForms recreate every handle, the WebView2 host included.src/BloomE2E/AUTOMATION-DEBT.md.user.config.The monitor number is not the one Windows Settings shows
Nothing can reproduce those numbers.
QueryDisplayConfig(QDC_ONLY_ACTIVE_PATHS), the API that is supposed to give them, returns the paths in exactly theScreen.AllScreensorder; the\.\DISPLAY<n>device names do not match either; and Microsoft documents no rule. Measured on one three-monitor machine, Windows Settings said 1 (primary, centre), 2 (right), 3 (left), while both APIs said 1 (right), 2 (left), 3 (primary, centre).So the number counts left to right, which follows the arrangement picture a developer can read even though it is not the numbers printed on it. This changes what an existing variable means: master indexed
Screen.AllScreens, the order of the display drivers. Bloom also writes the whole mapping to%TEMP%\SIL\Bloom\Log.txton every automation start, so a developer can read the number instead of guessing it.Tests
src/BloomTests/AutomationWindowPlacementTests.cscovers the variable in 24 tests: every value that names a monitor,headlessin four casings,0, the unusable values, the left-to-right ordering, the off-every-monitor bounds against however many monitors the machine has, and that a run which is not automation ignores the variable.Recorded rather than done
Two pieces of work this raised are written up in
src/BloomE2E/AUTOMATION-DEBT.mdinstead of being done here:Verified against a real Bloom as well:
BLOOM_AUTOMATION_MONITOR=2put the window on the primary monitor,0put it off every monitor, and the tab test passed both times.This change is
Devin review