Skip to content

Fix debugging task test SDK state leak - #12603

Merged
jonathanpeppers merged 3 commits into
mainfrom
jonathanpeppers-fix-java-home-test-leak
Sep 1, 2026
Merged

Fix debugging task test SDK state leak#12603
jonathanpeppers merged 3 commits into
mainfrom
jonathanpeppers-fix-java-home-test-leak

Conversation

@jonathanpeppers

@jonathanpeppers jonathanpeppers commented Aug 31, 2026

Copy link
Copy Markdown
Member

Description

DebuggingTasksTests.TestResolveToolsExists invokes ResolveXamarinAndroidTools with temporary Android and Java SDK directories. On Windows, AndroidSdkWindows.Initialize() updates process-level JAVA_HOME and PATH, and AndroidSdk.Refresh() leaves shared SDK state pointing at the temporary fixtures. After those directories are deleted, later tests such as PackagingTest.CheckSignApk(False,False,NativeAOT) can launch Android tooling with the stale JDK path.

Capture the real SDK paths and original process environment immediately before task execution. A focused try/finally around the two task calls restores AndroidSdk, JAVA_HOME, and PATH; the existing test setup and cache assertions remain otherwise unchanged.

This is a standalone fix for the deterministic Windows CI contamination observed in PR #12554 build 1571801; it does not modify or depend on the Dependabot change.

Testing

  • dotnet-local.cmd build src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Xamarin.Android.Build.Tests.csproj -v:quiet
  • dotnet-local.cmd test bin\TestDebug\net10.0\Xamarin.Android.Build.Tests.dll --filter "Name~TestResolveToolsExists" -v:quiet

Restore process environment variables and Xamarin.AndroidTools SDK singletons after TestResolveToolsExists uses temporary SDK fixtures.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 31, 2026 14:17
Keep the existing test setup intact and restore only JAVA_HOME, PATH, and AndroidSdk state after executing the task.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

Pull request overview

Fixes a deterministic Windows test-suite contamination where DebuggingTasksTests.TestResolveToolsExists mutates process environment variables and shared Xamarin.AndroidTools SDK singletons, leaving later tests with stale SDK/JDK paths after the test’s temporary directories are deleted.

Changes:

  • Snapshot JAVA_HOME/PATH and Android/MonoDroid SDK singleton state before running ResolveXamarinAndroidTools.
  • Restore SDK singletons and environment variables in a finally block to prevent cross-test leakage.
  • Add post-restore assertions (within the repeated test) to verify state is reset after each run.
File summaries
File Description
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/DebuggingTasksTests.cs Adds state capture + finally restoration of Android/MonoDroid SDK singletons and JAVA_HOME/PATH, with assertions to prevent future state leaks.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

Keep existing assertions outside the cleanup block so the final diff only wraps the calls that mutate process-wide SDK state.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers jonathanpeppers added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Aug 31, 2026
@jonathanpeppers
jonathanpeppers merged commit ef93ac2 into main Sep 1, 2026
43 of 44 checks passed
@jonathanpeppers
jonathanpeppers deleted the jonathanpeppers-fix-java-home-test-leak branch September 1, 2026 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants