Skip to content

[flaky-ci] Stabilize Gradle Facebook device coverage #12719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
simonrozsival wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[flaky-ci] Stabilize Gradle Facebook device coverage #12719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
28eb2f7
tests: stabilize Gradle Facebook device test
simonrozsival Sep 8, 2026
154b387
tests: scope Maven probe failure classification
simonrozsival Sep 8, 2026
0c411d8
[Java.Interop] make Maven cache writes atomic
simonrozsival Sep 8, 2026
c312e52
Merge origin/main into GradleFBProj stability
simonrozsival Sep 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 48 additions & 11 deletions external/Java.Interop/src/Java.Interop.Tools.Maven/Repositories/CachedMavenRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,21 @@ public bool TryGetFilePath (Artifact artifact, string filename, [NotNullWhen (tr
}

if (repository.TryGetFile (artifact, filename, out var repo_stream)) {
Directory.CreateDirectory (GetArtifactDirectory (artifact));

using (var sw = File.Create (file))
using (repo_stream)
repo_stream.CopyTo (sw);
var directory = GetArtifactDirectory (artifact);
Directory.CreateDirectory (directory);
var temporary_file = Path.Combine (directory, Path.GetRandomFileName ());

try {
using (var sw = File.Create (temporary_file))
using (repo_stream)
repo_stream.CopyTo (sw);

PublishTemporaryFile (temporary_file, file);
} catch (Exception ex) {
DeleteTemporaryFileAfterFailure (temporary_file, ex);
throw;
}
File.Delete (temporary_file);

path = file;
return true;
Expand All @@ -69,19 +79,46 @@ public bool TryGetFilePath (Artifact artifact, string filename, [NotNullWhen (tr
return file;

if (repository.TryGetFile (artifact, filename, out var repo_stream)) {
Directory.CreateDirectory (GetArtifactDirectory (artifact));

using (var sw = File.Create (file))
using (repo_stream)
await repo_stream.CopyToAsync (sw, 81920, cancellationToken);

var directory = GetArtifactDirectory (artifact);
Directory.CreateDirectory (directory);
var temporary_file = Path.Combine (directory, Path.GetRandomFileName ());

try {
using (var sw = File.Create (temporary_file))
using (repo_stream)
await repo_stream.CopyToAsync (sw, 81920, cancellationToken);

PublishTemporaryFile (temporary_file, file);
} catch (Exception ex) {
DeleteTemporaryFileAfterFailure (temporary_file, ex);
throw;
}
File.Delete (temporary_file);

return file;
}

return null;
}

static void PublishTemporaryFile (string temporaryFile, string file)
{
try {
File.Move (temporaryFile, file);

@jonathanpeppers jonathanpeppers Sep 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it possible this could move across volumes? Z:\temp to C:\Users\jon\...

I think that this can fail in that case on Windows.

Copilot thinks maybe the temp file should be in the target directory and use FileMode.CreateNew so it would throw if a file with the same name exists already.

} catch (IOException) when (File.Exists (file)) {
// Another process completed the same artifact download first.
}
Comment on lines +108 to +110

@jonathanpeppers jonathanpeppers Sep 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we treat this as a success? What if the other process copied the wrong file contents?

}

static void DeleteTemporaryFileAfterFailure (string temporaryFile, Exception failure)
{
try {
File.Delete (temporaryFile);
} catch (Exception cleanupException) when (cleanupException is IOException || cleanupException is UnauthorizedAccessException) {
failure.Data ["MavenCacheTemporaryFileCleanupException"] = cleanupException;
}
Comment on lines +117 to +119

@jonathanpeppers jonathanpeppers Sep 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

did copilot do this "to avoid empty catch blocks"?

is there a way it could be logged instead?

}

/// <summary>
/// Returns the on-disk path where the given <paramref name="artifact"/> + <paramref name="filename"/>
/// would be cached under <see cref="CacheDirectory"/>. Does not download or check for existence.
Expand Down
132 changes: 129 additions & 3 deletions external/Java.Interop/tests/Java.Interop.Tools.Maven-Tests/CachedMavenRepositoryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,71 @@ public void TryGetFilePath_HappyPath_DownloadsAndReturnsExpectedPath ()
CollectionAssert.AreEqual (content, File.ReadAllBytes (path!));
}

[Test]
public void TryGetFilePath_FailedDownloadDoesNotPopulateCache ()
{
var artifact = new Artifact ("com.example", "lib", "1.0.0");
var content = new byte [] { 1, 2, 3 };
var inner = new FlakyRepository ("central", content);
var cache = new CachedMavenRepository (cache_dir, inner);
var path = cache.GetArtifactFilePath (artifact, "lib-1.0.0.jar");
var directory = Path.GetDirectoryName (path);
if (directory is null)
throw new InvalidOperationException ($"Could not determine the directory for '{path}'.");

Assert.Throws<IOException> (() => cache.TryGetFilePath (artifact, "lib-1.0.0.jar", out _));
Assert.IsFalse (File.Exists (path), "A failed download must not populate the final cache path.");
CollectionAssert.IsEmpty (Directory.GetFiles (directory), "A failed download must clean up its temporary file.");

Assert.IsTrue (cache.TryGetFilePath (artifact, "lib-1.0.0.jar", out var actual));
Assert.AreEqual (path, actual);
CollectionAssert.AreEqual (content, File.ReadAllBytes (path));
Assert.AreEqual (2, inner.CallCount);
}

[Test]
public async Task GetFilePathAsync_FailedDownloadDoesNotPopulateCache ()
{
var artifact = new Artifact ("com.example", "lib", "1.0.0");
var content = new byte [] { 1, 2, 3 };
var inner = new FlakyRepository ("central", content);
var cache = new CachedMavenRepository (cache_dir, inner);
var path = cache.GetArtifactFilePath (artifact, "lib-1.0.0.jar");
var directory = Path.GetDirectoryName (path);
if (directory is null)
throw new InvalidOperationException ($"Could not determine the directory for '{path}'.");

Assert.ThrowsAsync<IOException> (async () =>
await cache.GetFilePathAsync (artifact, "lib-1.0.0.jar", CancellationToken.None));
Assert.IsFalse (File.Exists (path), "A failed download must not populate the final cache path.");
CollectionAssert.IsEmpty (Directory.GetFiles (directory), "A failed download must clean up its temporary file.");

var actual = await cache.GetFilePathAsync (artifact, "lib-1.0.0.jar", CancellationToken.None);

Assert.AreEqual (path, actual);
CollectionAssert.AreEqual (content, File.ReadAllBytes (path));
Assert.AreEqual (2, inner.CallCount);
}

[Test]
public async Task GetFilePathAsync_ConcurrentPublisherUsesCompletedFile ()
{
var artifact = new Artifact ("com.example", "lib", "1.0.0");
var content = new byte [] { 1, 2, 3 };
var path = Path.GetFullPath (Path.Combine (cache_dir, "central", "com.example", "lib", "1.0.0", "lib-1.0.0.jar"));
var directory = Path.GetDirectoryName (path);
if (directory is null)
throw new InvalidOperationException ($"Could not determine the directory for '{path}'.");
var inner = new StubRepository ("central", artifact, "lib-1.0.0.jar", () => new PublishingStream (path, content));
var cache = new CachedMavenRepository (cache_dir, inner);

var actual = await cache.GetFilePathAsync (artifact, "lib-1.0.0.jar", CancellationToken.None);

Assert.AreEqual (path, actual);
CollectionAssert.AreEqual (content, File.ReadAllBytes (path));
CollectionAssert.AreEqual (new [] { path }, Directory.GetFiles (directory));
}

[Test]
public void GetArtifactFilePath_RelativeFilename_Throws ()
{
Expand Down Expand Up @@ -142,22 +207,27 @@ sealed class StubRepository : IMavenRepository
{
readonly Artifact expected;
readonly string expected_filename;
readonly byte [] content;
readonly Func<Stream> stream_factory;

public StubRepository (string name, Artifact expected, string filename, byte [] content)
: this (name, expected, filename, () => new MemoryStream (content))
{
}

public StubRepository (string name, Artifact expected, string filename, Func<Stream> streamFactory)
{
Name = name;
this.expected = expected;
this.expected_filename = filename;
this.content = content;
stream_factory = streamFactory;
}

public string Name { get; }

public bool TryGetFile (Artifact artifact, string filename, [NotNullWhen (true)] out Stream? stream)
{
if (artifact.GroupId == expected.GroupId && artifact.Id == expected.Id && artifact.Version == expected.Version && filename == expected_filename) {
stream = new MemoryStream (content);
stream = stream_factory ();
return true;
}
stream = null;
Expand All @@ -182,4 +252,60 @@ public bool TryGetFile (Artifact artifact, string filename, [NotNullWhen (true)]
throw new InvalidOperationException ("Inner repository should not be consulted when the resolved path escapes the cache directory.");
}
}

sealed class FlakyRepository : IMavenRepository
{
readonly byte [] content;

public FlakyRepository (string name, byte [] content)
{
Name = name;
this.content = content;
}

public string Name { get; }

public int CallCount { get; private set; }

public bool TryGetFile (Artifact artifact, string filename, [NotNullWhen (true)] out Stream? stream)
{
CallCount++;
stream = CallCount == 1 ? new FaultingStream () : new MemoryStream (content);
return true;
}
}

sealed class FaultingStream : MemoryStream
{
public override void CopyTo (Stream destination, int bufferSize)
{
destination.WriteByte (1);
throw new IOException ("Simulated interrupted Maven download.");
}

public override async Task CopyToAsync (Stream destination, int bufferSize, CancellationToken cancellationToken)
{
await destination.WriteAsync (new byte [] { 1 }, 0, 1, cancellationToken);
throw new IOException ("Simulated interrupted Maven download.");
}
}

sealed class PublishingStream : MemoryStream
{
readonly string path;
readonly byte [] content;

public PublishingStream (string path, byte [] content)
: base (content)
{
this.path = path;
this.content = content;
}

public override async Task CopyToAsync (Stream destination, int bufferSize, CancellationToken cancellationToken)
{
await base.CopyToAsync (destination, bufferSize, cancellationToken);
File.WriteAllBytes (path, content);
}
}
}
77 changes: 77 additions & 0 deletions ...amarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidGradleProjectTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,5 +630,82 @@ public void BindLibraryWithMultipleGradleVersions (string agpVersion, string gra
FileAssert.Exists (Path.Combine (Root, builder.ProjectDirectory, proj.OutputPath, $"{moduleName}-release.aar"));
}

[TestCase (
"Plugin [id: 'com.android.application', version: '9.1.1'] was not found\n" +
"Searched in dotnet-public-maven(https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1)",
"AGP plugin resolution from dotnet-public-maven"
)]
[TestCase (
"Could not GET an AndroidX module from dotnet-public-maven\nConnection reset",
"connection reset"
)]
[TestCase (
"Could not HEAD an AGP plugin in dotnet-public-maven\npkgs.dev.azure.com: nodename nor servname provided, or not known",
"DNS resolution failure"
)]
[TestCase (
"error XA4236: Cannot download Maven artifact 'com.facebook.android:facebook-bolts'.\n" +
"facebook-bolts-18.3.0.aar: nodename nor servname provided, or not known (pkgs.dev.azure.com:443)",
"DNS resolution failure"
)]
[TestCase (
"error XA4236: Cannot download Maven artifact 'com.facebook.android:facebook-bolts'.\n" +
"error XA4236: - facebook-bolts-18.3.0.jar: Response status code does not indicate success: 401 (Unauthorized)\n" +
"error XA4236: - facebook-bolts-18.3.0.aar: nodename nor servname provided, or not known (pkgs.dev.azure.com:443)",
"DNS resolution failure"
)]
[TestCase (
"error XA4236: Cannot download Maven artifact 'com.facebook.android:facebook-bolts'.\n" +
"error XA4236: - facebook-bolts-18.3.0.jar: Response status code does not indicate success: 404 (Not Found)\n" +
"error XA4236: - facebook-bolts-18.3.0.aar: Connection reset",
"connection reset"
)]
[TestCase (
"Failed to install the following SDK components: platforms;android-37.0\nOperation timed out",
"network timeout"
)]
public void DetectTransientDependencyResolutionFailure (string buildOutput, string expectedReason)
{
Assert.IsTrue (TransientBuildFailure.TryGetDependencyResolutionReason (buildOutput.Split ('\n'), out string reason));
Assert.AreEqual (expectedReason, reason);
}

[TestCase ("error CS1002: ; expected")]
[TestCase ("error XAGRDL1000: 'Invalid' not found in root project")]
[TestCase (
"Plugin [id: 'com.android.application', version: '99.0.0'] was not found\n" +
"Searched in MavenCentral"
)]
[TestCase (
"Could not GET an artifact from dotnet-public-maven\n" +
"Response status code does not indicate success: 404 (Not Found)"
)]
[TestCase (
"error XA4236: Cannot download Maven artifact 'com.facebook.android:facebook-bolts'.\n" +
"error XA4236: - facebook-bolts-18.3.0.jar: Connection reset\n" +
"error XA4236: - facebook-bolts-18.3.0.aar: Response status code does not indicate success: 401 (Unauthorized)"
)]
[TestCase (
"error XA4236: Cannot download Maven artifact 'com.facebook.android:facebook-bolts'.\n" +
"error XA4236: - facebook-bolts-18.3.0.jar: Response status code does not indicate success: 404 (Not Found)\n" +
"error XA4236: - facebook-bolts-18.3.0.aar: Response status code does not indicate success: 404 (Not Found)"
)]
[TestCase (
"error XA4236: Cannot download Maven artifact 'com.example:jar-only'.\n" +
"error XA4236: - jar-only-1.0.jar: Response status code does not indicate success: 404 (Not Found)"
)]
[TestCase (
"error XA4236: Cannot download Maven artifact 'com.example:jar-only'.\n" +
"error XA4236: - jar-only-1.0.jar: Response status code does not indicate success: 404 (Not Found)\n" +
"error XA4236: Cannot download Maven artifact 'com.facebook.android:facebook-bolts'.\n" +
"error XA4236: - facebook-bolts-18.3.0.jar: Response status code does not indicate success: 404 (Not Found)\n" +
"error XA4236: - facebook-bolts-18.3.0.aar: Connection reset"
)]
public void DoNotClassifyPermanentBuildFailureAsTransient (string buildOutput)
{
Assert.IsFalse (TransientBuildFailure.TryGetDependencyResolutionReason (buildOutput.Split ('\n'), out string reason));
Assert.AreEqual ("", reason);
}

}
}
Loading
Loading