[tests] Add DotNetRunCtrlC integration test#11293
[tests] Add DotNetRunCtrlC integration test#11293jonathanpeppers wants to merge 1 commit intomainfrom
Conversation
f65d066 to
996bb8f
Compare
|
Note that this doesn't actually fix: I believe #11264 occurs on Windows only -- I'm still investigating. |
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
Pull request overview
Adds a new device integration test to validate that sending Ctrl+C/SIGINT to dotnet run (with WaitForExit=true / Microsoft.Android.Run) triggers the tool’s cancellation handler and stops the running app on the device/emulator.
Changes:
- Add
DotNetRunCtrlCdevice integration test that launches an app viadotnet run, sends Ctrl+C, and verifies both tool output and app termination. - Add
Process.SendCtrlC()test utility (Unix/macOS) to send SIGINT to a process and its descendants. - Adjust
Microsoft.Android.Runto return exit code130onOperationCanceledException(Ctrl+C-style cancellation), plus a small doc comment update inDotNetCLI.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs | Adds the new DotNetRunCtrlC device integration test and log capture/attachment. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Utilities/ProcessExtensions.cs | Introduces SendCtrlC() helper to deliver SIGINT to dotnet run + descendants on Unix/macOS. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/DotNetCLI.cs | Doc comment update for ExecuteProcess optional working directory parameter. |
| src/Microsoft.Android.Run/Program.cs | Returns 130 when cancellation bubbles as OperationCanceledException. |
There was a problem hiding this comment.
✅ LGTM — Clean, well-structured test addition
Summary: This PR adds a device integration test (DotNetRunCtrlC) that verifies Ctrl+C (SIGINT) properly stops an Android app launched via dotnet run, along with a SendCtrlC() extension method using libc kill() P/Invoke, and a small OperationCanceledException handler in Microsoft.Android.Run/Program.cs.
What looks good
- The
OperationCanceledException→ exit code 130 inProgram.csis correct (standard Unix convention) and properly ordered before the catch-allExceptionhandler. SendCtrlC()is well-documented, references the dotnet/sdk pattern, handles the full process tree viapgreprecursion, and has an appropriatePlatformNotSupportedExceptionguard for Windows.- The test follows the established
DotNetRunWaitForExitpattern closely — same project setup, output capture via async handlers,try/finallycleanup withprocess.Kill(entireProcessTree: true), log file attachment. - Good layered assertions: verify app is running → send signal → verify exit → verify "Stopping application..." → verify app stopped on device.
Issues found
| Severity | Count |
|---|---|
| 💡 Suggestion | 2 |
CI Status
CI is still in progress (Xamarin.Android-PR internal pipeline is pending). No failures detected so far — dotnet-android and license/cla checks passed.
Generated by Android PR Reviewer for issue #11293 · ● 1.9M
996bb8f to
212f498
Compare
|
Addressed in latest push: pidof race condition (line 253): Good catch — force-stop in finally (line 261): Added The other comments are not relevant for this PR:
|
Add a device integration test that verifies Ctrl+C (SIGINT) sent to
`dotnet run` properly stops the Android app on the device/emulator.
The test:
1. Builds and launches an Android app via `dotnet run` with WaitForExit
2. Waits for app logcat output confirming launch
3. Verifies the app PID exists on the device
4. Sends SIGINT via Process.SendCtrlC() extension method
5. Asserts the process exits gracefully
6. Verifies Microsoft.Android.Run's Ctrl+C handler ran ("Stopping application...")
7. Confirms the app is no longer running on the device
Also adds a `SendCtrlC()` extension method on `Process` in
`ProcessExtensions.cs`. On Unix/macOS, it uses `pgrep` to find all
descendant processes and sends SIGINT to the entire process tree,
simulating what a real terminal Ctrl+C does (SIGINT to the whole
foreground process group). Without this, child processes like
`Microsoft.Android.Run` (launched by `dotnet run` via MSBuild) would
not receive the signal.
Fixes a bug in `Microsoft.Android.Run` where `OperationCanceledException`
from `GetAppPidAsync()` would escape through the top-level catch block
when Ctrl+C was pressed, printing "Error: The operation was canceled."
instead of exiting cleanly.
Fixes: #11264
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
212f498 to
66c28ba
Compare
Adds a device integration test that verifies Ctrl+C (SIGINT) sent to
dotnet runproperly stops the Android app on the device/emulator.Test flow:
dotnet runwithWaitForExit=trueadb shell pidofprocess.SendCtrlC()extension methodMicrosoft.Android.Run's Ctrl+C handler ran ("Stopping application...")Also adds a
SendCtrlC()extension method onProcessinProcessExtensions.cs, using libckill(pid, SIGINT)P/Invoke (Unix/macOS only; throwsPlatformNotSupportedExceptionon Windows).