From ce177390b19f8625f340e425493514e6696e0715 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 5 Jul 2025 17:48:09 +0200 Subject: [PATCH 01/25] ci(ui-tests): use a better way to call AutoHotKey By using the `/ErrorStdOut` flag (which required `| Out-Default` because `AutoHotKey64.exe` is a GUI program and would therefore not show any stderr otherwise), we can avoid those blocking dialogs that are so hard to diagnose when running AutoHotKey in a GitHub workflow. Signed-off-by: Johannes Schindelin --- .github/workflows/ui-tests.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index f063ed7596..2d26a73078 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -84,11 +84,11 @@ jobs: - name: Run UI tests id: ui-tests run: | - $p = Start-Process -PassThru -FilePath "${env:RUNNER_TEMP}\ahk\AutoHotKey64.exe" -ArgumentList ui-tests\background-hook.ahk, "$PWD\bg-hook" - $p.WaitForExit() - if ($p.ExitCode -ne 0) { echo "::error::Test failed!" } else { echo "::notice::Test log" } + $exitCode = 0 + & "${env:RUNNER_TEMP}\ahk\AutoHotKey64.exe" /ErrorStdOut /force ui-tests\background-hook.ahk "$PWD\bg-hook" 2>&1 | Out-Default + if (!$?) { $exitCode = 1; echo "::error::Test failed!" } else { echo "::notice::Test log" } type bg-hook.log - if ($p.ExitCode -ne 0) { exit 1 } + exit $exitCode - name: Show logs, if canceled if: cancelled() run: type bg-hook.log From cf7a164257bc772c26d9f4a630c407a829fc9ae6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 5 Jul 2025 13:54:23 +0200 Subject: [PATCH 02/25] ci(ui-tests): upload the test logs The test logs are quite interesting to have, and not only those: In case of a fatal failure, the test directory is valuable information, too. Let's always upload them as build artifacts. For convenience, let's just reuse the `ui-tests/` directory as the place to put all of those files; Technically, we do not need the files in there that are tracked by Git, but practically speaking, it is neat to have them packaged in the same `.zip` file as the test logs and stuff. Signed-off-by: Johannes Schindelin --- .github/workflows/ui-tests.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 2d26a73078..265263adde 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -83,12 +83,20 @@ jobs: ui-tests - name: Run UI tests id: ui-tests + working-directory: ui-tests run: | $exitCode = 0 - & "${env:RUNNER_TEMP}\ahk\AutoHotKey64.exe" /ErrorStdOut /force ui-tests\background-hook.ahk "$PWD\bg-hook" 2>&1 | Out-Default + & "${env:RUNNER_TEMP}\ahk\AutoHotKey64.exe" /ErrorStdOut /force background-hook.ahk "$PWD\bg-hook" 2>&1 | Out-Default if (!$?) { $exitCode = 1; echo "::error::Test failed!" } else { echo "::notice::Test log" } type bg-hook.log exit $exitCode - name: Show logs, if canceled if: cancelled() + working-directory: ui-tests run: type bg-hook.log + - name: Upload test results + if: always() + uses: actions/upload-artifact@v4 + with: + name: ui-tests-${{ matrix.os }} + path: ui-tests From 41d3ae0ebe9caf665d1ed0329574134d8be2bdbc Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Jul 2025 11:48:15 +0200 Subject: [PATCH 03/25] ci(ui-tests): take a screenshot when canceled Sometimes the logs are empty and it is highly unclear what has happened. In such a scenario, a picture is indeed worth more than a thousand words. Note that this commit is more complicated than anyone would like, for two reasons: - While PowerShell is the right tool for the job, a PowerShell step in GitHub Actions will pop up a Terminal window, _hiding_ what we want to screenshot. To work around that, I tried to run things in a Bash step. _Also_ opens a Terminal window! Node.js to the rescue. - _Of course_ it is complicated to take a screenshot. The challenge is to figure out the dimensions of the screen, which should be as easy as looking at `[System.Windows.Forms.Screen]::PrimaryScreen`'s `Bounds` attribute. Easy peasy, right? No, it's not. Most machines nowadays have a _ridiculous_ resolution which is why most setups have a _zoom factor_. Getting to that factor should be trivial, by calling `GetDeviceCaps(hDC, LOGPIXELSX)`, but that's not working in modern Windows! There is a per-monitor display scaling ("DPI"). But even _that_ is hard to get at, calling `GetDpiForMonitor()` will still return 96 DPI (i.e. 100% zoom) because PowerShell is not marked as _Per-Monitor DPI Aware_. Since we do not want to write a manifest into the same directory as `powershell.exe` resides, we have to jump through yet another hoop to get that. Signed-off-by: Johannes Schindelin --- .github/workflows/ui-tests.yml | 57 ++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 265263adde..385db799b8 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -94,6 +94,63 @@ jobs: if: cancelled() working-directory: ui-tests run: type bg-hook.log + - name: Take screenshot, if canceled + id: take-screenshot + if: cancelled() || failure() + shell: powershell + run: | + Add-Type -TypeDefinition @" + using System; + using System.Runtime.InteropServices; + + public class DpiHelper { + [DllImport("user32.dll")] + public static extern bool SetProcessDpiAwarenessContext(IntPtr dpiContext); + + [DllImport("Shcore.dll")] + public static extern int GetDpiForMonitor(IntPtr hmonitor, int dpiType, out uint dpiX, out uint dpiY); + + [DllImport("User32.dll")] + public static extern IntPtr MonitorFromPoint(System.Drawing.Point pt, uint dwFlags); + + [DllImport("user32.dll")] + public static extern bool ShowWindow(IntPtr hWnd, int nCmdShow); + + public static uint GetDPI() { + // DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2 = -4 + SetProcessDpiAwarenessContext((IntPtr)(-4)); + + uint dpiX, dpiY; + IntPtr monitor = MonitorFromPoint(new System.Drawing.Point(0, 0), 2); // MONITOR_DEFAULTTONEAREST + GetDpiForMonitor(monitor, 0, out dpiX, out dpiY); // MDT_EFFECTIVE_DPI + return (dpiX + dpiY) / 2; + } + } + "@ -ReferencedAssemblies "System.Drawing.dll" + + # First, minimize the Console window in which this script is running + $hwnd = (Get-Process -Id $PID).MainWindowHandle + $SW_MINIMIZE = 6 + + [DpiHelper]::ShowWindow($hwnd, $SW_MINIMIZE) + + # Now, get the DPI + $dpi = [DpiHelper]::GetDPI() + + # This function takes a screenshot and saves it as a PNG file + [Reflection.Assembly]::LoadWithPartialName("System.Drawing") + function screenshot([Drawing.Rectangle]$bounds, $path) { + $bmp = New-Object Drawing.Bitmap $bounds.width, $bounds.height + $graphics = [Drawing.Graphics]::FromImage($bmp) + $graphics.CopyFromScreen($bounds.Location, [Drawing.Point]::Empty, $bounds.size) + $bmp.Save($path) + $graphics.Dispose() + $bmp.Dispose() + } + Add-Type -AssemblyName System.Windows.Forms + $screen = [System.Windows.Forms.Screen]::PrimaryScreen + $bounds = [Drawing.Rectangle]::FromLTRB(0, 0, $screen.Bounds.Width * $dpi / 96, $screen.Bounds.Height * $dpi / 96) + screenshot $bounds "ui-tests/screenshot.png" - name: Upload test results if: always() uses: actions/upload-artifact@v4 From 8d098c50576bc5a0fb3b178336d657fcde27cd8a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Jul 2025 11:49:17 +0200 Subject: [PATCH 04/25] ci(ui-tests): enforce a sensible timeout The `ui-tests` job currently takes around half a minute to a minute. Therefore it is quite alarming when the job times out after the default of 6h. Let's let it time out after 10 minutes (we can always increase it when more tests are added). Unfortunately, this changes the failure mode and `cancelled()` will no longer return `true`. Which means that the existing `if:` clauses have to be adapted for showing the logs and taking & uploading a screenshot. While thinking of the logs, I realized that it would be good also to have them in the case that the run succeeded, so let's always show them. Signed-off-by: Johannes Schindelin --- .github/workflows/ui-tests.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 385db799b8..0d84182aca 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -83,6 +83,7 @@ jobs: ui-tests - name: Run UI tests id: ui-tests + timeout-minutes: 10 working-directory: ui-tests run: | $exitCode = 0 @@ -90,8 +91,8 @@ jobs: if (!$?) { $exitCode = 1; echo "::error::Test failed!" } else { echo "::notice::Test log" } type bg-hook.log exit $exitCode - - name: Show logs, if canceled - if: cancelled() + - name: Show logs + if: always() working-directory: ui-tests run: type bg-hook.log - name: Take screenshot, if canceled From d287c6b7d88eba88452538fe9f77901910b5646b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 Jul 2025 20:47:09 +0200 Subject: [PATCH 05/25] ui-tests: convert DOS line endings to Unix ones Git is still totally unable to represent DOS line endings gracefully, so let's just follow the German wisdom that between two parties, the wise one gives in. Signed-off-by: Johannes Schindelin --- ui-tests/background-hook.ahk | 256 +++++++++++++++++------------------ 1 file changed, 128 insertions(+), 128 deletions(-) diff --git a/ui-tests/background-hook.ahk b/ui-tests/background-hook.ahk index 43f723b56d..0a7af3af54 100755 --- a/ui-tests/background-hook.ahk +++ b/ui-tests/background-hook.ahk @@ -1,129 +1,129 @@ -#Requires AutoHotkey v2.0 - -; This script is an integration test for the following scenario: -; A Git hook spawns a background process that outputs some text -; to the console even after Git has exited. - -; At some point in time, the Cygwin/MSYS2 runtime left the console -; in a state where it was not possible to navigate the history via -; CursorUp/Down, as reported in https://github.com/microsoft/git/issues/730. -; This was fixed in the Cygwin/MSYS2 runtime, but then regressed again. -; This test is meant to verify that the issue is fixed and remains so. - -; First, set the worktree path; This path will be reused -; for the `.log` file). -if A_Args.Length > 0 - workTree := A_Args[1] -else -{ - ; Create a unique worktree path in the TEMP directory. - workTree := EnvGet('TEMP') . '\git-test-background-hook' - if FileExist(workTree) - { - counter := 0 - while FileExist(workTree '-' counter) - counter++ - workTree := workTree '-' counter - } -} - -Info(text) { - FileAppend text '`n', workTree '.log' -} - -closeWindow := false -childPid := 0 -ExitWithError(error) { - Info 'Error: ' error - if closeWindow - WinClose "A" - else if childPid != 0 - ProcessClose childPid - ExitApp 1 -} - -RunWaitOne(command) { - shell := ComObject("WScript.Shell") - ; Execute a single command via cmd.exe - exec := shell.Exec(A_ComSpec " /C " command) - ; Read and return the command's output - return exec.StdOut.ReadAll() -} - -SetWorkingDir(EnvGet('TEMP')) -Info 'uname: ' RunWaitOne('uname -a') -Info RunWaitOne('git version --build-options') - -RunWait('git init "' workTree '"', '', 'Hide') -if A_LastError - ExitWithError 'Could not initialize Git worktree at: ' workTree - -SetWorkingDir(workTree) -if A_LastError - ExitWithError 'Could not set working directory to: ' workTree - -if not FileExist('.git/hooks') and not DirCreate('.git/hooks') - ExitWithError 'Could not create hooks directory: ' workTree - -FileAppend("#!/bin/sh`npowershell -command 'for ($i = 0; $i -lt 50; $i++) { echo $i; sleep -milliseconds 10 }' &`n", '.git/hooks/pre-commit') -if A_LastError - ExitWithError 'Could not create pre-commit hook: ' A_LastError - -Run 'wt.exe -d . ' A_ComSpec ' /d', , , &childPid -if A_LastError - ExitWithError 'Error launching CMD: ' A_LastError -Info 'Launched CMD: ' childPid -if not WinWait(A_ComSpec, , 9) - ExitWithError 'CMD window did not appear' -Info 'Got window' -WinActivate -CloseWindow := true -WinMove 0, 0 -Info 'Moved window to top left (so that the bottom is not cut off)' - -CaptureText() { - ControlGetPos &cx, &cy, &cw, &ch, 'Windows.UI.Composition.DesktopWindowContentBridge1', "A" - titleBarHeight := 54 - scrollBarWidth := 28 - pad := 8 - - SavedClipboard := ClipboardAll - A_Clipboard := '' - SendMode('Event') - MouseMove cx + pad, cy + titleBarHeight + pad - MouseClickDrag 'Left', , , cx + cw - scrollBarWidth, cy + ch - pad, , '' - MouseClick 'Right' - ClipWait() - Result := A_Clipboard - Clipboard := SavedClipboard - return Result -} - -Info('Setting committer identity') -Send('git config user.name Test{Enter}git config user.email t@e.st{Enter}') - -Info('Committing') -Send('git commit --allow-empty -m zOMG{Enter}') -; Wait for the hook to finish printing -While not RegExMatch(CaptureText(), '`n49$') -{ - Sleep 100 - if A_Index > 1000 - ExitWithError 'Timed out waiting for commit to finish' - MouseClick 'WheelDown', , , 20 -} -Info('Hook finished') - -; Verify that CursorUp shows the previous command -Send('{Up}') -Sleep 150 -Text := CaptureText() -if not RegExMatch(Text, 'git commit --allow-empty -m zOMG *$') - ExitWithError 'Cursor Up did not work: ' Text -Info('Match!') - -Send('^C') -Send('exit{Enter}') -Sleep 50 -SetWorkingDir(EnvGet('TEMP')) +#Requires AutoHotkey v2.0 + +; This script is an integration test for the following scenario: +; A Git hook spawns a background process that outputs some text +; to the console even after Git has exited. + +; At some point in time, the Cygwin/MSYS2 runtime left the console +; in a state where it was not possible to navigate the history via +; CursorUp/Down, as reported in https://github.com/microsoft/git/issues/730. +; This was fixed in the Cygwin/MSYS2 runtime, but then regressed again. +; This test is meant to verify that the issue is fixed and remains so. + +; First, set the worktree path; This path will be reused +; for the `.log` file). +if A_Args.Length > 0 + workTree := A_Args[1] +else +{ + ; Create a unique worktree path in the TEMP directory. + workTree := EnvGet('TEMP') . '\git-test-background-hook' + if FileExist(workTree) + { + counter := 0 + while FileExist(workTree '-' counter) + counter++ + workTree := workTree '-' counter + } +} + +Info(text) { + FileAppend text '`n', workTree '.log' +} + +closeWindow := false +childPid := 0 +ExitWithError(error) { + Info 'Error: ' error + if closeWindow + WinClose "A" + else if childPid != 0 + ProcessClose childPid + ExitApp 1 +} + +RunWaitOne(command) { + shell := ComObject("WScript.Shell") + ; Execute a single command via cmd.exe + exec := shell.Exec(A_ComSpec " /C " command) + ; Read and return the command's output + return exec.StdOut.ReadAll() +} + +SetWorkingDir(EnvGet('TEMP')) +Info 'uname: ' RunWaitOne('uname -a') +Info RunWaitOne('git version --build-options') + +RunWait('git init "' workTree '"', '', 'Hide') +if A_LastError + ExitWithError 'Could not initialize Git worktree at: ' workTree + +SetWorkingDir(workTree) +if A_LastError + ExitWithError 'Could not set working directory to: ' workTree + +if not FileExist('.git/hooks') and not DirCreate('.git/hooks') + ExitWithError 'Could not create hooks directory: ' workTree + +FileAppend("#!/bin/sh`npowershell -command 'for ($i = 0; $i -lt 50; $i++) { echo $i; sleep -milliseconds 10 }' &`n", '.git/hooks/pre-commit') +if A_LastError + ExitWithError 'Could not create pre-commit hook: ' A_LastError + +Run 'wt.exe -d . ' A_ComSpec ' /d', , , &childPid +if A_LastError + ExitWithError 'Error launching CMD: ' A_LastError +Info 'Launched CMD: ' childPid +if not WinWait(A_ComSpec, , 9) + ExitWithError 'CMD window did not appear' +Info 'Got window' +WinActivate +CloseWindow := true +WinMove 0, 0 +Info 'Moved window to top left (so that the bottom is not cut off)' + +CaptureText() { + ControlGetPos &cx, &cy, &cw, &ch, 'Windows.UI.Composition.DesktopWindowContentBridge1', "A" + titleBarHeight := 54 + scrollBarWidth := 28 + pad := 8 + + SavedClipboard := ClipboardAll + A_Clipboard := '' + SendMode('Event') + MouseMove cx + pad, cy + titleBarHeight + pad + MouseClickDrag 'Left', , , cx + cw - scrollBarWidth, cy + ch - pad, , '' + MouseClick 'Right' + ClipWait() + Result := A_Clipboard + Clipboard := SavedClipboard + return Result +} + +Info('Setting committer identity') +Send('git config user.name Test{Enter}git config user.email t@e.st{Enter}') + +Info('Committing') +Send('git commit --allow-empty -m zOMG{Enter}') +; Wait for the hook to finish printing +While not RegExMatch(CaptureText(), '`n49$') +{ + Sleep 100 + if A_Index > 1000 + ExitWithError 'Timed out waiting for commit to finish' + MouseClick 'WheelDown', , , 20 +} +Info('Hook finished') + +; Verify that CursorUp shows the previous command +Send('{Up}') +Sleep 150 +Text := CaptureText() +if not RegExMatch(Text, 'git commit --allow-empty -m zOMG *$') + ExitWithError 'Cursor Up did not work: ' Text +Info('Match!') + +Send('^C') +Send('exit{Enter}') +Sleep 50 +SetWorkingDir(EnvGet('TEMP')) DirDelete(workTree, true) \ No newline at end of file From 596d3dc1f5cc74cf0497963e04d54b07bb969c67 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 Jul 2025 20:51:48 +0200 Subject: [PATCH 06/25] ui-tests: ensure that `.ahk` scripts are stored with Unix line endings Signed-off-by: Johannes Schindelin --- ui-tests/.gitattributes | 1 + 1 file changed, 1 insertion(+) create mode 100644 ui-tests/.gitattributes diff --git a/ui-tests/.gitattributes b/ui-tests/.gitattributes new file mode 100644 index 0000000000..4dd1b9375b --- /dev/null +++ b/ui-tests/.gitattributes @@ -0,0 +1 @@ +*.ahk eol=lf From 4e375cc009a809cc51678c1f7d36d9220a7d8ff2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 Jul 2025 20:31:49 +0200 Subject: [PATCH 07/25] ui-tests: refactor setup into its own function Signed-off-by: Johannes Schindelin --- ui-tests/background-hook.ahk | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/ui-tests/background-hook.ahk b/ui-tests/background-hook.ahk index 0a7af3af54..4676d329bb 100755 --- a/ui-tests/background-hook.ahk +++ b/ui-tests/background-hook.ahk @@ -10,22 +10,26 @@ ; This was fixed in the Cygwin/MSYS2 runtime, but then regressed again. ; This test is meant to verify that the issue is fixed and remains so. -; First, set the worktree path; This path will be reused -; for the `.log` file). -if A_Args.Length > 0 - workTree := A_Args[1] -else -{ - ; Create a unique worktree path in the TEMP directory. - workTree := EnvGet('TEMP') . '\git-test-background-hook' - if FileExist(workTree) +SetWorkTree() { + global workTree + ; First, set the worktree path; This path will be reused + ; for the `.log` file). + if A_Args.Length > 0 + workTree := A_Args[1] + else { - counter := 0 - while FileExist(workTree '-' counter) - counter++ - workTree := workTree '-' counter + ; Create a unique worktree path in the TEMP directory. + workTree := EnvGet('TEMP') . '\git-test-background-hook' + if FileExist(workTree) + { + counter := 0 + while FileExist(workTree '-' counter) + counter++ + workTree := workTree '-' counter + } } } +SetWorkTree() Info(text) { FileAppend text '`n', workTree '.log' From 9b6f4029d4a24bd71c16e52156647733b3b74aac Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 Jul 2025 20:36:58 +0200 Subject: [PATCH 08/25] ui-tests: rename `CaptureText()` to reflect that it's about Windows Terminal This function will soon be moved into a library, therefore it makes sense to give it a non-generic name. Signed-off-by: Johannes Schindelin --- ui-tests/background-hook.ahk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui-tests/background-hook.ahk b/ui-tests/background-hook.ahk index 4676d329bb..10960dcebf 100755 --- a/ui-tests/background-hook.ahk +++ b/ui-tests/background-hook.ahk @@ -85,7 +85,7 @@ CloseWindow := true WinMove 0, 0 Info 'Moved window to top left (so that the bottom is not cut off)' -CaptureText() { +CaptureTextFromWindowsTerminal() { ControlGetPos &cx, &cy, &cw, &ch, 'Windows.UI.Composition.DesktopWindowContentBridge1', "A" titleBarHeight := 54 scrollBarWidth := 28 @@ -109,7 +109,7 @@ Send('git config user.name Test{Enter}git config user.email t@e.st{Enter}') Info('Committing') Send('git commit --allow-empty -m zOMG{Enter}') ; Wait for the hook to finish printing -While not RegExMatch(CaptureText(), '`n49$') +While not RegExMatch(CaptureTextFromWindowsTerminal(), '`n49$') { Sleep 100 if A_Index > 1000 @@ -121,7 +121,7 @@ Info('Hook finished') ; Verify that CursorUp shows the previous command Send('{Up}') Sleep 150 -Text := CaptureText() +Text := CaptureTextFromWindowsTerminal() if not RegExMatch(Text, 'git commit --allow-empty -m zOMG *$') ExitWithError 'Cursor Up did not work: ' Text Info('Match!') From e33eefe1ed15d7842a000f20e329452595aaa7cf Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 Jul 2025 20:42:24 +0200 Subject: [PATCH 09/25] ui-tests: Move reusable functions into a library These functions will be used in an upcoming test script to verify that Ctrl+C does what it is supposed to do. Signed-off-by: Johannes Schindelin --- ui-tests/background-hook.ahk | 61 +---------------------------------- ui-tests/ui-test-library.ahk | 62 ++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 60 deletions(-) create mode 100644 ui-tests/ui-test-library.ahk diff --git a/ui-tests/background-hook.ahk b/ui-tests/background-hook.ahk index 10960dcebf..35ba52dea9 100755 --- a/ui-tests/background-hook.ahk +++ b/ui-tests/background-hook.ahk @@ -1,4 +1,5 @@ #Requires AutoHotkey v2.0 +#Include ui-test-library.ahk ; This script is an integration test for the following scenario: ; A Git hook spawns a background process that outputs some text @@ -10,50 +11,8 @@ ; This was fixed in the Cygwin/MSYS2 runtime, but then regressed again. ; This test is meant to verify that the issue is fixed and remains so. -SetWorkTree() { - global workTree - ; First, set the worktree path; This path will be reused - ; for the `.log` file). - if A_Args.Length > 0 - workTree := A_Args[1] - else - { - ; Create a unique worktree path in the TEMP directory. - workTree := EnvGet('TEMP') . '\git-test-background-hook' - if FileExist(workTree) - { - counter := 0 - while FileExist(workTree '-' counter) - counter++ - workTree := workTree '-' counter - } - } -} SetWorkTree() -Info(text) { - FileAppend text '`n', workTree '.log' -} - -closeWindow := false -childPid := 0 -ExitWithError(error) { - Info 'Error: ' error - if closeWindow - WinClose "A" - else if childPid != 0 - ProcessClose childPid - ExitApp 1 -} - -RunWaitOne(command) { - shell := ComObject("WScript.Shell") - ; Execute a single command via cmd.exe - exec := shell.Exec(A_ComSpec " /C " command) - ; Read and return the command's output - return exec.StdOut.ReadAll() -} - SetWorkingDir(EnvGet('TEMP')) Info 'uname: ' RunWaitOne('uname -a') Info RunWaitOne('git version --build-options') @@ -85,24 +44,6 @@ CloseWindow := true WinMove 0, 0 Info 'Moved window to top left (so that the bottom is not cut off)' -CaptureTextFromWindowsTerminal() { - ControlGetPos &cx, &cy, &cw, &ch, 'Windows.UI.Composition.DesktopWindowContentBridge1', "A" - titleBarHeight := 54 - scrollBarWidth := 28 - pad := 8 - - SavedClipboard := ClipboardAll - A_Clipboard := '' - SendMode('Event') - MouseMove cx + pad, cy + titleBarHeight + pad - MouseClickDrag 'Left', , , cx + cw - scrollBarWidth, cy + ch - pad, , '' - MouseClick 'Right' - ClipWait() - Result := A_Clipboard - Clipboard := SavedClipboard - return Result -} - Info('Setting committer identity') Send('git config user.name Test{Enter}git config user.email t@e.st{Enter}') diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk new file mode 100644 index 0000000000..827888e994 --- /dev/null +++ b/ui-tests/ui-test-library.ahk @@ -0,0 +1,62 @@ +; Reusable library functions for the UI tests. + +SetWorkTree() { + global workTree + ; First, set the worktree path; This path will be reused + ; for the `.log` file). + if A_Args.Length > 0 + workTree := A_Args[1] + else + { + ; Create a unique worktree path in the TEMP directory. + workTree := EnvGet('TEMP') . '\git-test-background-hook' + if FileExist(workTree) + { + counter := 0 + while FileExist(workTree '-' counter) + counter++ + workTree := workTree '-' counter + } + } +} + +Info(text) { + FileAppend text '`n', workTree '.log' +} + +closeWindow := false +childPid := 0 +ExitWithError(error) { + Info 'Error: ' error + if closeWindow + WinClose "A" + else if childPid != 0 + ProcessClose childPid + ExitApp 1 +} + +RunWaitOne(command) { + shell := ComObject("WScript.Shell") + ; Execute a single command via cmd.exe + exec := shell.Exec(A_ComSpec " /C " command) + ; Read and return the command's output + return exec.StdOut.ReadAll() +} + +CaptureTextFromWindowsTerminal() { + ControlGetPos &cx, &cy, &cw, &ch, 'Windows.UI.Composition.DesktopWindowContentBridge1', "A" + titleBarHeight := 54 + scrollBarWidth := 28 + pad := 8 + + SavedClipboard := ClipboardAll + A_Clipboard := '' + SendMode('Event') + MouseMove cx + pad, cy + titleBarHeight + pad + MouseClickDrag 'Left', , , cx + cw - scrollBarWidth, cy + ch - pad, , '' + MouseClick 'Right' + ClipWait() + Result := A_Clipboard + Clipboard := SavedClipboard + return Result +} \ No newline at end of file From fd9bd8276ad0b4bfd5b4cad16d0bcde3eb3ab153 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 Jul 2025 21:17:21 +0200 Subject: [PATCH 10/25] ui-tests: let SetWorkTree() accept a directory base name This base name should be different for the different UI tests (once we have more than one) so that they do not clash. Signed-off-by: Johannes Schindelin --- ui-tests/background-hook.ahk | 2 +- ui-tests/ui-test-library.ahk | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ui-tests/background-hook.ahk b/ui-tests/background-hook.ahk index 35ba52dea9..24a41ccfba 100755 --- a/ui-tests/background-hook.ahk +++ b/ui-tests/background-hook.ahk @@ -11,7 +11,7 @@ ; This was fixed in the Cygwin/MSYS2 runtime, but then regressed again. ; This test is meant to verify that the issue is fixed and remains so. -SetWorkTree() +SetWorkTree('git-test-background-hook') SetWorkingDir(EnvGet('TEMP')) Info 'uname: ' RunWaitOne('uname -a') diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk index 827888e994..264e3586c4 100644 --- a/ui-tests/ui-test-library.ahk +++ b/ui-tests/ui-test-library.ahk @@ -1,6 +1,6 @@ ; Reusable library functions for the UI tests. -SetWorkTree() { +SetWorkTree(defaultName) { global workTree ; First, set the worktree path; This path will be reused ; for the `.log` file). @@ -9,7 +9,7 @@ SetWorkTree() { else { ; Create a unique worktree path in the TEMP directory. - workTree := EnvGet('TEMP') . '\git-test-background-hook' + workTree := EnvGet('TEMP') . '\' . defaultName if FileExist(workTree) { counter := 0 From ab61d99f73486bacd70c33e9041e7350b83dd02c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 Jul 2025 21:19:36 +0200 Subject: [PATCH 11/25] ui-tests: create the Git worktree as part of SetWorkTree() And likewise, have a tear-down function (CleanUpWorkTree()). Signed-off-by: Johannes Schindelin --- ui-tests/background-hook.ahk | 15 +-------------- ui-tests/ui-test-library.ahk | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/ui-tests/background-hook.ahk b/ui-tests/background-hook.ahk index 24a41ccfba..54e12442df 100755 --- a/ui-tests/background-hook.ahk +++ b/ui-tests/background-hook.ahk @@ -13,18 +13,6 @@ SetWorkTree('git-test-background-hook') -SetWorkingDir(EnvGet('TEMP')) -Info 'uname: ' RunWaitOne('uname -a') -Info RunWaitOne('git version --build-options') - -RunWait('git init "' workTree '"', '', 'Hide') -if A_LastError - ExitWithError 'Could not initialize Git worktree at: ' workTree - -SetWorkingDir(workTree) -if A_LastError - ExitWithError 'Could not set working directory to: ' workTree - if not FileExist('.git/hooks') and not DirCreate('.git/hooks') ExitWithError 'Could not create hooks directory: ' workTree @@ -70,5 +58,4 @@ Info('Match!') Send('^C') Send('exit{Enter}') Sleep 50 -SetWorkingDir(EnvGet('TEMP')) -DirDelete(workTree, true) \ No newline at end of file +CleanUpWorkTree() \ No newline at end of file diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk index 264e3586c4..c9379b29bd 100644 --- a/ui-tests/ui-test-library.ahk +++ b/ui-tests/ui-test-library.ahk @@ -18,6 +18,25 @@ SetWorkTree(defaultName) { workTree := workTree '-' counter } } + + SetWorkingDir(EnvGet('TEMP')) + Info 'uname: ' RunWaitOne('uname -a') + Info RunWaitOne('git version --build-options') + + RunWait('git init "' workTree '"', '', 'Hide') + if A_LastError + ExitWithError 'Could not initialize Git worktree at: ' workTree + + SetWorkingDir(workTree) + if A_LastError + ExitWithError 'Could not set working directory to: ' workTree +} + +CleanUpWorkTree() { + global workTree + SetWorkingDir(EnvGet('TEMP')) + Info 'Cleaning up worktree: ' workTree + DirDelete(workTree, true) } Info(text) { From febbeefb1487c5ced693a7d8c8b99df86e99b2f5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 5 Jul 2025 13:35:52 +0200 Subject: [PATCH 12/25] ui-tests: document how `CaptureTextFromWindowsTerminal()` works It is a tricky (and somewhat fragile) part of the testing framework, let's document what it does, how, and why it's done this way. Signed-off-by: Johannes Schindelin --- ui-tests/ui-test-library.ahk | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk index c9379b29bd..d73cdf4e94 100644 --- a/ui-tests/ui-test-library.ahk +++ b/ui-tests/ui-test-library.ahk @@ -62,6 +62,17 @@ RunWaitOne(command) { return exec.StdOut.ReadAll() } +; This function is quite the hack. It assumes that the Windows Terminal is the active window, +; then drags the mouse diagonally across the window to select all text and then copies it. +; +; This is fragile! If any other window becomes active, or if the mouse is moved, +; the function will not work as intended. +; +; An alternative would be to use `ControlSend`, e.g. +; `ControlSend '+^a', 'Windows.UI.Input.InputSite.WindowClass1', 'ahk_id ' . hwnd +; This _kinda_ works, the text is selected (all text, in fact), but the PowerShell itself +; _also_ processes the keyboard events and therefore they leave ugly and unintended +; `^Ac` characters in the prompt. So that alternative is not really usable. CaptureTextFromWindowsTerminal() { ControlGetPos &cx, &cy, &cw, &ch, 'Windows.UI.Composition.DesktopWindowContentBridge1', "A" titleBarHeight := 54 From f58efbe08b1a786cbd4f786056793fa272216d53 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 Jul 2025 22:47:48 +0200 Subject: [PATCH 13/25] ui-tests: move code to wait for text in the Windows Terminal into a function This code is way too useful _not_ to make it reusable. While at it, improve on the timeout logic to be quite a bit more precise. Signed-off-by: Johannes Schindelin --- ui-tests/background-hook.ahk | 9 +-------- ui-tests/ui-test-library.ahk | 13 +++++++++++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/ui-tests/background-hook.ahk b/ui-tests/background-hook.ahk index 54e12442df..af7c27d313 100755 --- a/ui-tests/background-hook.ahk +++ b/ui-tests/background-hook.ahk @@ -38,14 +38,7 @@ Send('git config user.name Test{Enter}git config user.email t@e.st{Enter}') Info('Committing') Send('git commit --allow-empty -m zOMG{Enter}') ; Wait for the hook to finish printing -While not RegExMatch(CaptureTextFromWindowsTerminal(), '`n49$') -{ - Sleep 100 - if A_Index > 1000 - ExitWithError 'Timed out waiting for commit to finish' - MouseClick 'WheelDown', , , 20 -} -Info('Hook finished') +WaitForRegExInWindowsTerminal('`n49$', 'Timed out waiting for commit to finish', 'Hook finished', 100000) ; Verify that CursorUp shows the previous command Send('{Up}') diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk index d73cdf4e94..2d1ffc1be5 100644 --- a/ui-tests/ui-test-library.ahk +++ b/ui-tests/ui-test-library.ahk @@ -89,4 +89,17 @@ CaptureTextFromWindowsTerminal() { Result := A_Clipboard Clipboard := SavedClipboard return Result +} + +WaitForRegExInWindowsTerminal(regex, errorMessage, successMessage, timeout := 5000) { + timeout := timeout + A_TickCount + ; Wait for the regex to match in the terminal output + while not RegExMatch(CaptureTextFromWindowsTerminal(), regex) + { + Sleep 100 + if A_TickCount > timeout + ExitWithError errorMessage + MouseClick 'WheelDown', , , 20 + } + Info(successMessage) } \ No newline at end of file From cbb3b37fc1896952111fcda772e89baf2c1fc944 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 Jul 2025 22:44:46 +0200 Subject: [PATCH 14/25] ui-tests: fix indentation of `RunWaitOne()` It used only 3 spaces, when 4 were called for. Signed-off-by: Johannes Schindelin --- ui-tests/ui-test-library.ahk | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk index 2d1ffc1be5..a20d6b15ac 100644 --- a/ui-tests/ui-test-library.ahk +++ b/ui-tests/ui-test-library.ahk @@ -55,11 +55,11 @@ ExitWithError(error) { } RunWaitOne(command) { - shell := ComObject("WScript.Shell") - ; Execute a single command via cmd.exe - exec := shell.Exec(A_ComSpec " /C " command) - ; Read and return the command's output - return exec.StdOut.ReadAll() + shell := ComObject("WScript.Shell") + ; Execute a single command via cmd.exe + exec := shell.Exec(A_ComSpec " /C " command) + ; Read and return the command's output + return exec.StdOut.ReadAll() } ; This function is quite the hack. It assumes that the Windows Terminal is the active window, From 3ae005bdc28bb8d9b0ba06ba049f7d2e38952166 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 3 Jul 2025 22:45:33 +0200 Subject: [PATCH 15/25] ui-tests: improve `RunWaitOne()` There is actually no need to have a flashing window just to capture the output of the command: using the trick (use `Run()`, pipe to `clip` and then use `A_Clipboard`) from https://stackoverflow.com/a/32298415 makes it possible to hide the console window _and_ capture the output. Signed-off-by: Johannes Schindelin --- ui-tests/ui-test-library.ahk | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk index a20d6b15ac..0193b5b47b 100644 --- a/ui-tests/ui-test-library.ahk +++ b/ui-tests/ui-test-library.ahk @@ -55,11 +55,14 @@ ExitWithError(error) { } RunWaitOne(command) { + SavedClipboard := ClipboardAll shell := ComObject("WScript.Shell") ; Execute a single command via cmd.exe - exec := shell.Exec(A_ComSpec " /C " command) + exec := shell.Run(A_ComSpec " /C " command " | clip", 0, true) ; Read and return the command's output - return exec.StdOut.ReadAll() + Result := A_Clipboard + Clipboard := SavedClipboard + return Result } ; This function is quite the hack. It assumes that the Windows Terminal is the active window, From 0e5a76a2e75e519f788137b4082825f471de69d0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Jul 2025 01:14:10 +0200 Subject: [PATCH 16/25] ui-tests: fix the `uname -a` invocation The `uname` executable is not on the `PATH` by default. Let's use the Git alias trick to execute it, then. Signed-off-by: Johannes Schindelin --- ui-tests/ui-test-library.ahk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk index 0193b5b47b..62182dcce4 100644 --- a/ui-tests/ui-test-library.ahk +++ b/ui-tests/ui-test-library.ahk @@ -20,7 +20,7 @@ SetWorkTree(defaultName) { } SetWorkingDir(EnvGet('TEMP')) - Info 'uname: ' RunWaitOne('uname -a') + Info 'uname: ' RunWaitOne('git -c alias.uname="!uname" uname -a') Info RunWaitOne('git version --build-options') RunWait('git init "' workTree '"', '', 'Hide') From ef6a27258c8452e8cd5c9442505f86238d41a8d5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Jul 2025 01:16:12 +0200 Subject: [PATCH 17/25] ui-tests: do not ignore errors in `RunWaitOne()` When the command fails, we should not simply continue, mistaking an empty string for being the valid output of a successful run. Signed-off-by: Johannes Schindelin --- ui-tests/ui-test-library.ahk | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk index 62182dcce4..dddfee11d3 100644 --- a/ui-tests/ui-test-library.ahk +++ b/ui-tests/ui-test-library.ahk @@ -59,6 +59,8 @@ RunWaitOne(command) { shell := ComObject("WScript.Shell") ; Execute a single command via cmd.exe exec := shell.Run(A_ComSpec " /C " command " | clip", 0, true) + if exec != 0 + ExitWithError 'Error executing command: ' command ; Read and return the command's output Result := A_Clipboard Clipboard := SavedClipboard From 0ba85df1691697a101484b9abd615a3a81847508 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Jul 2025 01:32:51 +0200 Subject: [PATCH 18/25] ui-tests: verify that a `sleep` in Windows Terminal can be interrupted The Ctrl+C way to interrupt run-away processes is highly important. It was recently broken in multiple ways in the Cygwin runtime (and hence also in the MSYS2 runtime). Let's add some integration tests that will catch regressions. It is admittedly less than ideal to add _integration_ tests; While imitating exactly what the end user does looks appealing at first, excellent tests impress by how quickly they allow regressions not only to be identified but also to be fixed. Even worse: all integration tests, by virtue of working in a broader environment than, say, unit tests, incur the price of sometimes catching unactionable bugs, i.e. bugs in software that is both outside of our control as well as not the target of our testing at all. Nevertheless, seeing as Cygwin did not add any unit tests for those Ctrl+C fixes (which is understandable, given how complex testing for Ctrl+C without UI testing would be), it is better to have integration tests than no tests at all. So here goes: This commit introduces a test that verifies that the MSYS2 `sleep.exe` can be interrupted when run from PowerShell in a Windows Terminal. This was broken in v3.6.0 and fixed in 7674c51e18 (Cygwin: console: Set ENABLE_PROCESSED_INPUT when disable_master_thread, 2025-07-01). Signed-off-by: Johannes Schindelin --- .github/workflows/ui-tests.yml | 7 +++++- ui-tests/ctrl-c.ahk | 46 ++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 ui-tests/ctrl-c.ahk diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 0d84182aca..922d5874f8 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -90,11 +90,16 @@ jobs: & "${env:RUNNER_TEMP}\ahk\AutoHotKey64.exe" /ErrorStdOut /force background-hook.ahk "$PWD\bg-hook" 2>&1 | Out-Default if (!$?) { $exitCode = 1; echo "::error::Test failed!" } else { echo "::notice::Test log" } type bg-hook.log + & "${env:RUNNER_TEMP}\ahk\AutoHotKey64.exe" /ErrorStdOut /force ctrl-c.ahk "$PWD\ctrl-c" 2>&1 | Out-Default + if (!$?) { $exitCode = 1; echo "::error::Ctrl+C Test failed!" } else { echo "::notice::Ctrl+C Test log" } + type ctrl-c.log exit $exitCode - name: Show logs if: always() working-directory: ui-tests - run: type bg-hook.log + run: | + type bg-hook.log + type ctrl-c.log - name: Take screenshot, if canceled id: take-screenshot if: cancelled() || failure() diff --git a/ui-tests/ctrl-c.ahk b/ui-tests/ctrl-c.ahk new file mode 100644 index 0000000000..10c580c218 --- /dev/null +++ b/ui-tests/ctrl-c.ahk @@ -0,0 +1,46 @@ +#Requires AutoHotkey v2.0 +#Include ui-test-library.ahk + +SetWorkTree('git-test-ctrl-c') + +powerShellPath := EnvGet('SystemRoot') . '\System32\WindowsPowerShell\v1.0\powershell.exe' +Run 'wt.exe -d . "' powerShellPath '"', , , &childPid +if A_LastError + ExitWithError 'Error launching PowerShell: ' A_LastError +Info 'Launched PowerShell: ' childPid +; Sadly, `WinWait('ahk_pid ' childPid)` does not work because the Windows Terminal window seems +; to be owned by the `wt.exe` process that launched. +; +; Probably should use the trick mentioned in +; https://www.autohotkey.com/boards/viewtopic.php?p=580081&sid=a40d0ce73efff728ffa6b4573dff07b9#p580081 +; where the `before` variable is assigned `WinGetList(winTitle).Length` before the `Run` command, +; and a `Loop` is used to wait until [`WinGetList()`](https://www.autohotkey.com/docs/v2/lib/WinGetList.htm) +; returns a different length, in which case the first array element is the new window. +; +; Also: This is crying out loud to be refactored into a function and then also used in `background-hook.ahk`! +hwnd := WinWait(powerShellPath, , 9) +if not hwnd + ExitWithError 'PowerShell window did not appear' +Info 'Got window' +WinActivate +CloseWindow := true +WinMove 0, 0 +Info 'Moved window to top left (so that the bottom is not cut off)' + +; sleep test +Sleep 1500 +; The `:;` is needed to force Git to call this via the shell, otherwise `/usr/bin/` would not resolve. +Send('git -c alias.sleep="{!}:;/usr/bin/sleep" sleep 15{Enter}') +Sleep 500 +; interrupt sleep; Ideally we'd call `Send('^C')` but that would too quick on GitHub Actions' runners. +; The idea for this work-around comes from https://www.reddit.com/r/AutoHotkey/comments/aok10s/comment/eg57e81/. +Send '{Ctrl down}{c down}' +Sleep 50 +Send '{c up}{Ctrl up}' +Sleep 150 +; Wait for the `^C` tell-tale that is the PowerShell prompt to appear +WaitForRegExInWindowsTerminal('>[ `n`r]*$', 'Timed out waiting for interrupt', 'Sleep was interrupted as desired') + +Send('exit{Enter}') +Sleep 50 +CleanUpWorkTree() \ No newline at end of file From 59f828912edde4a64bb3e80a8e680d385f67c957 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Jul 2025 01:17:19 +0200 Subject: [PATCH 19/25] ui-tests: trim the output in `RunWaitOne()` This imitates Unix shell's `$(...)` behavior better and is eminently more useful when capturing, say, the output of `cygpath`. Signed-off-by: Johannes Schindelin --- ui-tests/ui-test-library.ahk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk index dddfee11d3..01bda5abb7 100644 --- a/ui-tests/ui-test-library.ahk +++ b/ui-tests/ui-test-library.ahk @@ -61,8 +61,8 @@ RunWaitOne(command) { exec := shell.Run(A_ComSpec " /C " command " | clip", 0, true) if exec != 0 ExitWithError 'Error executing command: ' command - ; Read and return the command's output - Result := A_Clipboard + ; Read and return the command's output, trimming trailing newlines. + Result := RegExReplace(A_Clipboard, '`r?`n$', '') Clipboard := SavedClipboard return Result } From 99f35a8349d424a6317eb958e6cc4e5d29a367c6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Jul 2025 01:26:18 +0200 Subject: [PATCH 20/25] ui-tests: prepare `WaitForRegExInWindowsTerminal()` for disruptions I want to add an integration test that clones a repository via SSH. Unfortunately, I have not found a better way than to run OpenSSH for Windows' `sshd.exe` in debug mode, in which case every successful connection will open a terminal window. Let's be prepared for these annoying extra windows by forcefully activating the desired window as needed. This change is admittedly a bit intrusive; I did not find any more elegant way to achieve the same result, though. Signed-off-by: Johannes Schindelin --- ui-tests/ui-test-library.ahk | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk index 01bda5abb7..120e85ed76 100644 --- a/ui-tests/ui-test-library.ahk +++ b/ui-tests/ui-test-library.ahk @@ -78,7 +78,9 @@ RunWaitOne(command) { ; This _kinda_ works, the text is selected (all text, in fact), but the PowerShell itself ; _also_ processes the keyboard events and therefore they leave ugly and unintended ; `^Ac` characters in the prompt. So that alternative is not really usable. -CaptureTextFromWindowsTerminal() { +CaptureTextFromWindowsTerminal(winTitle := '') { + if winTitle != '' + WinActivate winTitle ControlGetPos &cx, &cy, &cw, &ch, 'Windows.UI.Composition.DesktopWindowContentBridge1', "A" titleBarHeight := 54 scrollBarWidth := 28 @@ -87,8 +89,14 @@ CaptureTextFromWindowsTerminal() { SavedClipboard := ClipboardAll A_Clipboard := '' SendMode('Event') + if winTitle != '' + WinActivate winTitle MouseMove cx + pad, cy + titleBarHeight + pad + if winTitle != '' + WinActivate winTitle MouseClickDrag 'Left', , , cx + cw - scrollBarWidth, cy + ch - pad, , '' + if winTitle != '' + WinActivate winTitle MouseClick 'Right' ClipWait() Result := A_Clipboard @@ -96,14 +104,21 @@ CaptureTextFromWindowsTerminal() { return Result } -WaitForRegExInWindowsTerminal(regex, errorMessage, successMessage, timeout := 5000) { +WaitForRegExInWindowsTerminal(regex, errorMessage, successMessage, timeout := 5000, winTitle := '') { timeout := timeout + A_TickCount ; Wait for the regex to match in the terminal output - while not RegExMatch(CaptureTextFromWindowsTerminal(), regex) + while true { + capturedText := CaptureTextFromWindowsTerminal(winTitle) + if RegExMatch(capturedText, regex) + break Sleep 100 - if A_TickCount > timeout + if A_TickCount > timeout { + Info('Captured text:`n' . capturedText) ExitWithError errorMessage + } + if winTitle != '' + WinActivate winTitle MouseClick 'WheelDown', , , 20 } Info(successMessage) From 22ed4519609db9c0c4d9120aed3ef558ab36a4c9 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Jul 2025 01:44:33 +0200 Subject: [PATCH 21/25] ui-tests: verify that interrupting clones via SSH works This was the actual use case that was broken and necessitated the fix in 7674c51e18 (Cygwin: console: Set ENABLE_PROCESSED_INPUT when disable_master_thread, 2025-07-01). It does require an SSH server, which Git for Windows no longer ships. Therefore, this test uses the `sshd.exe` of OpenSSH for Windows (https://github.com/powershell/Win32-OpenSSH) in conjunction with Git for Windows' `ssh.exe` (because using OpenSSH for Windows' variant of `ssh.exe` would not exercise the MSYS2 runtime and therefore not demonstrate a regression, should it surface in the future). To avoid failing the test because OpenSSH for Windows is not available, the test case is guarded by the environment variable `OPENSSH_FOR_WINDOWS_DIRECTORY` which needs to point to a directory that contains a working `sshd.exe`. Signed-off-by: Johannes Schindelin --- .github/workflows/ui-tests.yml | 22 +++++++++ ui-tests/ctrl-c.ahk | 87 ++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 922d5874f8..48865f7d1e 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -10,6 +10,7 @@ on: env: AUTOHOTKEY_VERSION: 2.0.19 WT_VERSION: 1.22.11141.0 + WIN32_OPENSSH_VERSION: 9.8.3.0p2-Preview jobs: ui-tests: @@ -76,6 +77,27 @@ jobs: "$WINDIR/system32/tar.exe" -C "$RUNNER_TEMP/ahk" -xf "$RUNNER_TEMP/ahk.zip" && cygpath -aw "$RUNNER_TEMP/ahk" >>$GITHUB_PATH - uses: actions/setup-node@v4 # the hook uses node for the background process + - uses: actions/cache/restore@v4 + id: restore-win32-openssh + with: + key: win32-openssh-${{ env.WIN32_OPENSSH_VERSION }} + path: ${{ runner.temp }}/win32-openssh.zip + - name: Download Win32-OpenSSH + if: steps.restore-win32-openssh.outputs.cache-hit != 'true' + shell: bash + run: | + curl -fLo "$RUNNER_TEMP/win32-openssh.zip" \ + https://github.com/PowerShell/Win32-OpenSSH/releases/download/v$WIN32_OPENSSH_VERSION/OpenSSH-Win64.zip + - uses: actions/cache/save@v4 + if: steps.restore-win32-openssh.outputs.cache-hit != 'true' + with: + key: win32-openssh-${{ env.WIN32_OPENSSH_VERSION }} + path: ${{ runner.temp }}/win32-openssh.zip + - name: Unpack Win32-OpenSSH + shell: bash + run: | + "$WINDIR/system32/tar.exe" -C "$RUNNER_TEMP" -xvf "$RUNNER_TEMP/win32-openssh.zip" && + echo "OPENSSH_FOR_WINDOWS_DIRECTORY=$(cygpath -aw "$RUNNER_TEMP/OpenSSH-Win64")" >>$GITHUB_ENV - uses: actions/checkout@v4 with: diff --git a/ui-tests/ctrl-c.ahk b/ui-tests/ctrl-c.ahk index 10c580c218..9e382ede7a 100644 --- a/ui-tests/ctrl-c.ahk +++ b/ui-tests/ctrl-c.ahk @@ -41,6 +41,93 @@ Sleep 150 ; Wait for the `^C` tell-tale that is the PowerShell prompt to appear WaitForRegExInWindowsTerminal('>[ `n`r]*$', 'Timed out waiting for interrupt', 'Sleep was interrupted as desired') +; Clone via SSH test; Requires an OpenSSH for Windows `sshd.exe` whose directory needs to be specified via +; the environment variable `OPENSSH_FOR_WINDOWS_DIRECTORY`. The clone will still be performed via Git's +; included `ssh.exe`, to exercise the MSYS2 runtime (which these UI tests are all about). + +openSSHPath := EnvGet('OPENSSH_FOR_WINDOWS_DIRECTORY') +if (openSSHPath != '' and FileExist(openSSHPath . '\sshd.exe')) { + Info('Generate 26M of data') + RunWait('git init --bare -b main large.git', '', 'Hide') + RunWait('git --git-dir=large.git -c alias.c="!(' . + 'printf \"reset refs/heads/main\\n\"; ' . + 'seq 100000 | ' . + 'sed \"s|.*|blob\\nmark :&\\ndata < 1234& +0000\\ndata <[ `n`r]*$', 'Timed out waiting for clone to be interrupted', 'clone was interrupted as desired') + + if DirExist(workTree . '\large-clone') + ExitWithError('`large-clone` was unexpectedly not deleted on interrupt') +} + Send('exit{Enter}') Sleep 50 CleanUpWorkTree() \ No newline at end of file From 0be36425eacb37e06748f2f7ac8537e3470fdc3e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 5 Jul 2025 20:01:34 +0200 Subject: [PATCH 22/25] ci(ui-tests): exclude the large repository from the build artifact In the previous commit, I added a new UI test that generates a somewhat large repository for testing the clone via SSH. Since that repository is created in the test directory, that would inflate the `ui-tests` build artifact rather dramatically. So let's create the repository outside of that directory. Signed-off-by: Johannes Schindelin --- .github/workflows/ui-tests.yml | 1 + ui-tests/ctrl-c.ahk | 15 ++++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 48865f7d1e..1b0f799f7d 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -112,6 +112,7 @@ jobs: & "${env:RUNNER_TEMP}\ahk\AutoHotKey64.exe" /ErrorStdOut /force background-hook.ahk "$PWD\bg-hook" 2>&1 | Out-Default if (!$?) { $exitCode = 1; echo "::error::Test failed!" } else { echo "::notice::Test log" } type bg-hook.log + $env:LARGE_FILES_DIRECTORY = "${env:RUNNER_TEMP}\large" & "${env:RUNNER_TEMP}\ahk\AutoHotKey64.exe" /ErrorStdOut /force ctrl-c.ahk "$PWD\ctrl-c" 2>&1 | Out-Default if (!$?) { $exitCode = 1; echo "::error::Ctrl+C Test failed!" } else { echo "::notice::Ctrl+C Test log" } type ctrl-c.log diff --git a/ui-tests/ctrl-c.ahk b/ui-tests/ctrl-c.ahk index 9e382ede7a..3150d82605 100644 --- a/ui-tests/ctrl-c.ahk +++ b/ui-tests/ctrl-c.ahk @@ -48,8 +48,13 @@ WaitForRegExInWindowsTerminal('>[ `n`r]*$', 'Timed out waiting for interrupt', ' openSSHPath := EnvGet('OPENSSH_FOR_WINDOWS_DIRECTORY') if (openSSHPath != '' and FileExist(openSSHPath . '\sshd.exe')) { Info('Generate 26M of data') - RunWait('git init --bare -b main large.git', '', 'Hide') - RunWait('git --git-dir=large.git -c alias.c="!(' . + largeFilesDirectory := EnvGet('LARGE_FILES_DIRECTORY') + if largeFilesDirectory == '' + largeFilesDirectory := workTree . '-large-files' + largeGitRepoPath := largeFilesDirectory . '\large.git' + largeGitClonePath := largeFilesDirectory . '\large-clone' + RunWait('git init --bare -b main "' . largeGitRepoPath . '"', '', 'Hide') + RunWait('git --git-dir="' . largeGitRepoPath . '" -c alias.c="!(' . 'printf \"reset refs/heads/main\\n\"; ' . 'seq 100000 | ' . 'sed \"s|.*|blob\\nmark :&\\ndata <[ `n`r]*$', 'Timed out waiting for clone to be interrupted', 'clone was interrupted as desired') - if DirExist(workTree . '\large-clone') + if DirExist(largeGitClonePath) ExitWithError('`large-clone` was unexpectedly not deleted on interrupt') } From 6673d8e1b8a1be0e06528735d5375e1891bbe997 Mon Sep 17 00:00:00 2001 From: Takashi Yano Date: Thu, 3 Jul 2025 10:51:09 +0900 Subject: [PATCH 23/25] Cygwin: console: Call set_input_mode() after changing disable_master_thread With the commit 476135a24506, set_input_mode() reffers to the flag disable_master_thread in tty::cygwin mode. So it is necessary to call set_input_mode() after changing disable_master_thread flag. However, the commit 476135a24506 was missing that. With this patch, set_input_mode() is called after changing the flag disable_master_thread, if the console input mode is tty::cygwin. Fixes: 476135a24506 ("Cygwin: console: Set ENABLE_PROCESSED_INPUT when disable_master_thread"); Signed-off-by: Takashi Yano Cherry-picked-from: 65a48c7202 (Cygwin: console: Call set_input_mode() after changing disable_master_thread, 2025-07-03) Signed-off-by: Johannes Schindelin --- winsup/cygwin/fhandler/console.cc | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc index 48c932647f..831df4f2cb 100644 --- a/winsup/cygwin/fhandler/console.cc +++ b/winsup/cygwin/fhandler/console.cc @@ -923,12 +923,12 @@ fhandler_console::cleanup_for_non_cygwin_app (handle_set_t *p) termios *ti = shared_console_info[unit] ? &(shared_console_info[unit]->tty_min_state.ti) : &dummy; /* Cleaning-up console mode for non-cygwin app. */ + set_disable_master_thread (con.owner == GetCurrentProcessId ()); /* conmode can be tty::restore when non-cygwin app is exec'ed from login shell. */ tty::cons_mode conmode = cons_mode_on_close (p); set_output_mode (conmode, ti, p); set_input_mode (conmode, ti, p); - set_disable_master_thread (con.owner == GetCurrentProcessId ()); } /* Return the tty structure associated with a given tty number. If the @@ -1121,8 +1121,8 @@ fhandler_console::bg_check (int sig, bool dontsignal) in the same process group. */ if (sig == SIGTTIN && con.curr_input_mode != tty::cygwin) { - set_input_mode (tty::cygwin, &tc ()->ti, get_handle_set ()); set_disable_master_thread (false, this); + set_input_mode (tty::cygwin, &tc ()->ti, get_handle_set ()); } if (sig == SIGTTOU && con.curr_output_mode != tty::cygwin) set_output_mode (tty::cygwin, &tc ()->ti, get_handle_set ()); @@ -1987,8 +1987,8 @@ fhandler_console::post_open_setup (int fd) /* Setting-up console mode for cygwin app started from non-cygwin app. */ if (fd == 0) { - set_input_mode (tty::cygwin, &get_ttyp ()->ti, &handle_set); set_disable_master_thread (false, this); + set_input_mode (tty::cygwin, &get_ttyp ()->ti, &handle_set); } else if (fd == 1 || fd == 2) set_output_mode (tty::cygwin, &get_ttyp ()->ti, &handle_set); @@ -2995,7 +2995,12 @@ fhandler_console::char_command (char c) if (con.args[i] == 1) /* DECCKM */ con.cursor_key_app_mode = (c == 'h'); if (con.args[i] == 9001) /* win32-input-mode (https://github.com/microsoft/terminal/blob/main/doc/specs/%234999%20-%20Improved%20keyboard%20handling%20in%20Conpty.md) */ - set_disable_master_thread (c == 'h', this); + { + set_disable_master_thread (c == 'h', this); + if (con.curr_input_mode == tty::cygwin) + set_input_mode (tty::cygwin, + &tc ()->ti, get_handle_set ()); + } } /* Call fix_tab_position() if screen has been alternated. */ if (need_fix_tab_position) From f6c4c7f446b69eadcce4a7589385a89a4e97c8ce Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Jul 2025 01:58:20 +0200 Subject: [PATCH 24/25] ui-tests: add `ping` interrupt test The fixes of 7674c51e18 (Cygwin: console: Set ENABLE_PROCESSED_INPUT when disable_master_thread, 2025-07-01) were unfortunately not complete; There were still a couple of edge cases where Ctrl+C was unable to interrupt processes. Let's add a demonstration of that issue. Signed-off-by: Johannes Schindelin --- ui-tests/ctrl-c.ahk | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ui-tests/ctrl-c.ahk b/ui-tests/ctrl-c.ahk index 3150d82605..4e14608180 100644 --- a/ui-tests/ctrl-c.ahk +++ b/ui-tests/ctrl-c.ahk @@ -41,6 +41,17 @@ Sleep 150 ; Wait for the `^C` tell-tale that is the PowerShell prompt to appear WaitForRegExInWindowsTerminal('>[ `n`r]*$', 'Timed out waiting for interrupt', 'Sleep was interrupted as desired') +; ping test (`cat.exe` should be interrupted, too) +Send('git -c alias.c="{!}cat | /c/windows/system32/ping -t localhost" c{Enter}') +Sleep 500 +WaitForRegExInWindowsTerminal('Pinging ', 'Timed out waiting for pinging to start', 'Pinging started') +Send('^C') ; interrupt ping and cat +Sleep 150 +; Wait for the `^C` tell-tale to appear +WaitForRegExInWindowsTerminal('Control-C', 'Timed out waiting for pinging to be interrupted', 'Pinging was interrupted as desired') +; Wait for the `^C` tell-tale that is the PowerShell prompt to appear +WaitForRegExInWindowsTerminal('>[ `n`r]*$', 'Timed out waiting for `cat.exe` to be interrupted', '`cat.exe` was interrupted as desired') + ; Clone via SSH test; Requires an OpenSSH for Windows `sshd.exe` whose directory needs to be specified via ; the environment variable `OPENSSH_FOR_WINDOWS_DIRECTORY`. The clone will still be performed via Git's ; included `ssh.exe`, to exercise the MSYS2 runtime (which these UI tests are all about). From 772caa703db5d9303ab6b24009e80a377ee00b94 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 5 Jul 2025 18:46:47 +0200 Subject: [PATCH 25/25] ui-tests: do verify the SSH hang fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In 0ae6a6fa74 (Cygwin: pipe: Fix SSH hang with non-cygwin pipe reader, 2025-06-27), a quite problematic bug was fixed where somewhat large-ish repositories could not be cloned via SSH anymore. This fix was not accompanied by a corresponding test case in Cygwin's test suite, i.e. there is no automated way to ensure that there won't be any regressions on that bug (and therefore it would fall onto end users to deal with those). This constitutes what Michael C. Feathers famously characterized as "legacy code" in his book "Working Effectively with Legacy Code": To me, legacy code is simply code without tests. I've gotten some grief for this definition. What do tests have to do with whether code is bad? To me, the answer is straightforward, and it is a point that I elaborate throughout the book: Code without tests is bad code. It doesn’t matter how well written it is; it doesn’t matter how pretty or object-oriented or well-encapsulated it is. With tests, we can change the behavior of our code quickly and verifiably. Without them, we really don’t know if our code is getting better or worse. Just to drive this point home, let me pull out Exhibit A: The bug fix in question, which is the latest (and hopefully last) commit in a _long_ chain of bug fixes that fix bugs introduced by preceding bug fixes: - 9e4d308cd5 (Cygwin: pipe: Adopt FILE_SYNCHRONOUS_IO_NONALERT flag for read pipe., 2021-11-10) fixed a bug where Cygwin hung by mistake while piping output from one .NET program as input to another .NET program (potentially introduced by 365199090c (Cygwin: pipe: Avoid false EOF while reading output of C# programs., 2021-11-07), which was itself a bug fix). It introduced a bug that was fixed by... - fc691d0246 (Cygwin: pipe: Make sure to set read pipe non-blocking for cygwin apps., 2024-03-11). Which introduced a bug that was purportedly fixed by... - 7ed9adb356 (Cygwin: pipe: Switch pipe mode to blocking mode by default, 2024-09-05). Which introduced a bug that was fixed by... - cbfaeba4f7 (Cygwin: pipe: Fix incorrect write length in raw_write(), 2024-11-06). Which introduced a bug that was fixed by... the SSH hang fix in 0ae6a6fa74 (Cygwin: pipe: Fix SSH hang with non-cygwin pipe reader, 2025-06-27). There is not only the common thread here that each of these bug fixes introduced a new bug, but also the common thread that none of the commits introduced new test cases into the test suite that could potentially have helped prevent future breakages in this code. So let's at least add an integration test here. Side note: I am quite unhappy with introducing integration tests. I know there are a lot of fans out there, but I cannot help wondering whether they favor the convenience of writing tests quickly over the vast cost of making debugging any regression a highly cumbersome and unenjoyable affair (try single-stepping through a test case that requires several processes to be orchestrated in unison). Also, integration tests have the large price of introducing moving parts outside the code base that is actually to be tested, opening the door for breakages caused by software (or infrastructure, think: network glitches!) that are completely outside the power or responsibility of the poor engineer tasked with fixing the breakages. Nevertheless, I have been unable despite days of trying to wrap my head around the issue to figure out a way to reproduce the `fhandler_pipe_fifo::raw_write()` hang without involving a MINGW `git.exe` and an MSYS2/Cygwin `ssh.exe`. So: It's the best I could do with any reasonable amount of effort. It's better to have integration tests that would demonstrate regressions than not having any tests for that at all. Signed-off-by: Johannes Schindelin --- ui-tests/ctrl-c.ahk | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/ui-tests/ctrl-c.ahk b/ui-tests/ctrl-c.ahk index 4e14608180..3ca8873de7 100644 --- a/ui-tests/ctrl-c.ahk +++ b/ui-tests/ctrl-c.ahk @@ -142,6 +142,23 @@ if (openSSHPath != '' and FileExist(openSSHPath . '\sshd.exe')) { if DirExist(largeGitClonePath) ExitWithError('`large-clone` was unexpectedly not deleted on interrupt') + + ; Now verify that the SSH-based clone actually works and does not hang + Info('Re-starting SSH server') + Run(openSSHPath . '\sshd.exe ' . sshdOptions, '', 'Hide', &sshdPID) + if A_LastError + ExitWithError 'Error starting SSH server: ' A_LastError + Info('Started SSH server: ' sshdPID) + + Info('Starting clone') + Send('git -c core.sshCommand="ssh ' . sshOptions . '" clone ' . cloneOptions . '{Enter}') + Sleep 500 + Info('Waiting for clone to finish') + WinActivate('ahk_id ' . hwnd) + WaitForRegExInWindowsTerminal('Receiving objects: .*, done\.`r?`nPS .*>[ `n`r]*$', 'Timed out waiting for clone to finish', 'Clone finished', 15000, 'ahk_id ' . hwnd) + + if not DirExist(largeGitClonePath) + ExitWithError('`large-clone` did not work?!?') } Send('exit{Enter}')