From 5c886b78165931c80294c2bb6609a9371626b633 Mon Sep 17 00:00:00 2001 From: Epsitha Ananth Date: Wed, 5 May 2021 09:05:06 -0700 Subject: [PATCH 1/9] tests --- ...osoft.DotNet.Build.Tasks.Feed.Tests.csproj | 4 + .../PublishToSymbolServerTest.cs | 164 +++++++++++++++++- .../TestInputs/Symbols/test.txt | 35 ++++ .../src/PublishArtifactsInManifestBase.cs | 22 ++- 4 files changed, 215 insertions(+), 10 deletions(-) create mode 100644 src/Microsoft.DotNet.Build.Tasks.Feed.Tests/TestInputs/Symbols/test.txt diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/Microsoft.DotNet.Build.Tasks.Feed.Tests.csproj b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/Microsoft.DotNet.Build.Tasks.Feed.Tests.csproj index d100e0d8321..528c18da1ef 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/Microsoft.DotNet.Build.Tasks.Feed.Tests.csproj +++ b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/Microsoft.DotNet.Build.Tasks.Feed.Tests.csproj @@ -38,4 +38,8 @@ Always + + + + diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs index 4b2784921bb..39986f5b993 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs +++ b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs @@ -1,8 +1,15 @@ +using System; using System.Collections.Generic; using System.IO; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.Arcade.Common; using Microsoft.Arcade.Test.Common; using Microsoft.DotNet.Build.Tasks.Feed.Model; +using Microsoft.DotNet.Build.Tasks.Feed.Tests.TestDoubles; using Microsoft.DotNet.Maestro.Client.Models; +using Microsoft.DotNet.VersionTools.BuildManifest.Model; using Xunit; namespace Microsoft.DotNet.Build.Tasks.Feed.Tests @@ -40,7 +47,6 @@ public void PublishToSymbolServersTest(SymbolTargetType symbolTargetType , strin Assert.True(test.Count == 1); } - [Fact] public void PublishToBothSymbolServerTest() { @@ -139,5 +145,161 @@ public void PublishSymbolApiIsCalledTest() false, false).IsCompleted); } + + [Fact] + public void SuccessfulDownloadFileTestAsync() + { + var buildEngine = new MockBuildEngine(); + var publishTask = new PublishArtifactsInManifestV3 + { + BuildEngine = buildEngine, + }; + + var testFile = Path.Combine("Symbols", "test.txt"); + var responseContent = TestInputs.ReadAllBytes(testFile); + var responses = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new ByteArrayContent(responseContent) + }; + + using HttpClient client = FakeHttpClient.WithResponse(responses); + var path = TestInputs.GetFullPath(Guid.NewGuid().ToString()); + + + var test = publishTask.DownloadFileAsync( + client, + PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts, + "1234", + "test.txt", + path); + + Assert.True(File.Exists(path)); + publishTask.DeleteTemporaryFiles(path); + publishTask.DeleteTemporaryDirectory(path); + } + + [Theory] + [InlineData(HttpStatusCode.BadRequest, 3)] + [InlineData(HttpStatusCode.NotFound, 2)] + [InlineData(HttpStatusCode.GatewayTimeout, 1)] + public async Task FailedDownloadTestAsync(HttpStatusCode httpStatus, int maxRetry) + { + var buildEngine = new MockBuildEngine(); + var publishTask = new PublishArtifactsInManifestV3 + { + BuildEngine = buildEngine, + }; + var testFile = Path.Combine("Symbols", "test.txt"); + var responseContent = TestInputs.ReadAllBytes(testFile); + publishTask.RetryHandler = new ExponentialRetry() + { + DelayBase = 1, + MaxAttempts = maxRetry + }; + var responses = new[] + { + new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(HttpStatusCode.NotFound) + { + Content = new ByteArrayContent(responseContent) + } + }; + using HttpClient client = FakeHttpClient.WithResponses(responses); + var path = TestInputs.GetFullPath(Guid.NewGuid().ToString()); + + try + { + await publishTask.DownloadFileAsync( + client, + PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts, + "1234", + "test.txt", + path); + } + catch (Exception ex) + { + Assert.Contains($"Failed to download local file '{path}' after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", ex.Message); + } + } + + [Theory] + [InlineData(PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts, "1")] + [InlineData(PublishArtifactsInManifestBase.ArtifactName.PackageArtifacts, "1234")] + public async Task GetContainerIdTestAsync(PublishArtifactsInManifestBase.ArtifactName artifactName, string containerId) + { + var buildEngine = new MockBuildEngine(); + var publishTask = new PublishArtifactsInManifestV3 + { + BuildEngine = buildEngine, + }; + publishTask.BuildId = "1243456"; + var testPackageName = Path.Combine("Symbols", "test.txt"); + var responseContent = TestInputs.ReadAllBytes(testPackageName); + var responses = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new ByteArrayContent(responseContent) + }; + + using HttpClient client = FakeHttpClient.WithResponse(responses); + var test = await publishTask.GetContainerIdAsync( + client, + artifactName); + Assert.Equal(containerId, test); + } + + [Theory] + [InlineData(HttpStatusCode.BadRequest, 2)] + [InlineData(HttpStatusCode.NotFound, 3)] + public async Task ContainerIdErrorAsync(HttpStatusCode httpStatus, int maxAttempt) + { + var buildEngine = new MockBuildEngine(); + var publishTask = new PublishArtifactsInManifestV3 + { + BuildEngine = buildEngine, + }; + publishTask.BuildId = "1243456"; + publishTask.RetryHandler = new ExponentialRetry() + { + DelayBase = 1, + MaxAttempts = maxAttempt + }; + var testPackageName = Path.Combine("Symbols", "test.txt"); + var responseContent = TestInputs.ReadAllBytes(testPackageName); + var responses = new[] + { + new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(HttpStatusCode.NotFound) + { + Content = new ByteArrayContent(responseContent) + } + }; + + using HttpClient client = FakeHttpClient.WithResponses(responses); + try + { + await publishTask.GetContainerIdAsync( + client, + PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts); + } + catch (Exception ex) + { + Assert.Contains($"Failed to get container id after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", ex.Message); + } + } + } } diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/TestInputs/Symbols/test.txt b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/TestInputs/Symbols/test.txt new file mode 100644 index 00000000000..91eaf5a7328 --- /dev/null +++ b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/TestInputs/Symbols/test.txt @@ -0,0 +1,35 @@ +{ + "count": 1, + "value": [ + { + "id": 2124, + "name": "PackageArtifacts", + "source": "testSource", + "resource": { + "type": "Container", + "data": "#/1234/PackageArtifacts", + "properties": { + "localpath": "D:\\workspace\\_work\\1\\s\\artifacts\\output\\packages", + "artifactsize": "7644905" + }, + "url": "https://dev.azure.com/dnceng/url", + "downloadUrl": "https://dev.azure.com/dnceng/_apis/build/builds/buildId/artifacts?artifactName=PackageArtifacts&api-version=6.0&%24format=zip" + } + }, + { + "id": 2123, + "name": "BlobArtifacts", + "source": "testSource", + "resource": { + "type": "Container", + "data": "#/1/BlobArtifacts", + "properties": { + "localpath": "D:\\workspace\\_work\\1\\s\\artifacts\\output\\packages", + "artifactsize": "7644905" + }, + "url": "https://dev.azure.com/dnceng/_apis/build/builds/buildId/artifacts?artifactName=BlobArtifacts&api-version=6.0", + "downloadUrl": "https://dev.azure.com/dnceng/_apis/build/builds/buildId/artifacts?artifactName=PackageArtifacts&api-version=6.0&%24format=zip" + } + } + ] +} diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs index c1c17f0c057..91173a21216 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs +++ b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs @@ -176,13 +176,13 @@ public abstract class PublishArtifactsInManifestBase : Microsoft.Build.Utilities /// public int RetryDelayMilliseconds { get; set; } = 5000; - public readonly ExponentialRetry RetryHandler = new ExponentialRetry + public ExponentialRetry RetryHandler = new ExponentialRetry { MaxAttempts = 5, DelayBase = 2.5 // 2.5 ^ 5 = ~1.5 minutes max wait between retries }; - private enum ArtifactName + public enum ArtifactName { [Description("PackageArtifacts")] PackageArtifacts, @@ -392,7 +392,9 @@ public async Task PublishSymbolsUsingStreamingAsync( StringBuilder symbolLog = new StringBuilder(); symbolLog.AppendLine("Publishing Symbols to Symbol server: "); var symbolCategory = TargetFeedContentType.Symbols; - string containerId = await GetContainerIdAsync(ArtifactName.BlobArtifacts); + + using HttpClient httpClient = CreateAzdoClient(AzureDevOpsOrg, false, AzureProject); + string containerId = await GetContainerIdAsync(httpClient, ArtifactName.BlobArtifacts); if (Log.HasLoggedErrors) { @@ -820,7 +822,7 @@ public HttpClient CreateAzdoClient( /// /// If it is PackageArtifacts or BlobArtifacts /// ContainerId - private async Task GetContainerIdAsync(ArtifactName artifactName) + public async Task GetContainerIdAsync(HttpClient client, ArtifactName artifactName) { string uri = $"{AzureDevOpsBaseUrl}/{AzureDevOpsOrg}/{AzureProject}/_apis/build/builds/{BuildId}/artifacts?api-version={AzureDevOpsFeedsApiVersion}"; @@ -833,7 +835,6 @@ private async Task GetContainerIdAsync(ArtifactName artifactName) CancellationTokenSource timeoutTokenSource = new CancellationTokenSource(TimeSpan.FromMinutes(TimeoutInMinutes)); - using HttpClient client = CreateAzdoClient(AzureDevOpsOrg, false, AzureProject); using HttpRequestMessage getMessage = new HttpRequestMessage(HttpMethod.Get, uri); using HttpResponseMessage response = await client.GetAsync(uri, timeoutTokenSource.Token); @@ -882,7 +883,7 @@ private async Task GetContainerIdAsync(ArtifactName artifactName) /// ContainerId where the packageArtifact and BlobArtifacts are stored /// Name the file we are trying to download /// Path where the file is being downloaded - private async Task DownloadFileAsync( + public async Task DownloadFileAsync( HttpClient client, ArtifactName artifactName, string containerId, @@ -1126,7 +1127,8 @@ private async Task PublishPackagesUsingStreamingToAzdoNugetAsync( TargetFeedConfig feedConfig, SemaphoreSlim clientThrottle) { - string containerId = await GetContainerIdAsync(ArtifactName.PackageArtifacts); + using HttpClient httpClient = CreateAzdoClient(AzureDevOpsOrg, false, AzureProject); + string containerId = await GetContainerIdAsync(httpClient, ArtifactName.PackageArtifacts); if (Log.HasLoggedErrors) { @@ -1475,7 +1477,8 @@ private async Task PublishBlobsUsingStreamingToAzDoNugetAsync( TargetFeedConfig feedConfig, SemaphoreSlim clientThrottle) { - string containerId = await GetContainerIdAsync(ArtifactName.BlobArtifacts); + using HttpClient httpClient = CreateAzdoClient(AzureDevOpsOrg, false, AzureProject); + string containerId = await GetContainerIdAsync(httpClient, ArtifactName.BlobArtifacts); if (Log.HasLoggedErrors) { @@ -1632,7 +1635,8 @@ private async Task PublishBlobsToAzureStorageNugetUsingStreamingPublishingAsync( TargetFeedConfig feedConfig, SemaphoreSlim clientThrottle) { - string containerId = await GetContainerIdAsync(ArtifactName.BlobArtifacts); + using HttpClient httpClient = CreateAzdoClient(AzureDevOpsOrg, false, AzureProject); + string containerId = await GetContainerIdAsync(httpClient, ArtifactName.BlobArtifacts); if (Log.HasLoggedErrors) { From 4b162a3505ea1bcaaef65458b94d5788b1721b88 Mon Sep 17 00:00:00 2001 From: Epsitha Ananth Date: Wed, 5 May 2021 12:20:40 -0700 Subject: [PATCH 2/9] remove unwanted using --- .../PublishToSymbolServerTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs index 39986f5b993..ade111a012e 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs +++ b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs @@ -9,7 +9,6 @@ using Microsoft.DotNet.Build.Tasks.Feed.Model; using Microsoft.DotNet.Build.Tasks.Feed.Tests.TestDoubles; using Microsoft.DotNet.Maestro.Client.Models; -using Microsoft.DotNet.VersionTools.BuildManifest.Model; using Xunit; namespace Microsoft.DotNet.Build.Tasks.Feed.Tests From fbd126cd02e5f995401f3fa0326a215c0286c68f Mon Sep 17 00:00:00 2001 From: Epsitha Ananth Date: Fri, 18 Jun 2021 17:10:14 -0700 Subject: [PATCH 3/9] addressed feedback --- .../PublishToSymbolServerTest.cs | 81 +++++++++++++------ .../TestInputs/Symbols/test.txt | 66 +++++++-------- 2 files changed, 88 insertions(+), 59 deletions(-) diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs index ade111a012e..9057a4da09d 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs +++ b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs @@ -146,7 +146,7 @@ public void PublishSymbolApiIsCalledTest() } [Fact] - public void SuccessfulDownloadFileTestAsync() + public void DownloadFileAsyncSucceedsForValidUrl() { var buildEngine = new MockBuildEngine(); var publishTask = new PublishArtifactsInManifestV3 @@ -156,15 +156,14 @@ public void SuccessfulDownloadFileTestAsync() var testFile = Path.Combine("Symbols", "test.txt"); var responseContent = TestInputs.ReadAllBytes(testFile); - var responses = new HttpResponseMessage(HttpStatusCode.OK) + var response = new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(responseContent) }; - using HttpClient client = FakeHttpClient.WithResponse(responses); + using HttpClient client = FakeHttpClient.WithResponse(response); var path = TestInputs.GetFullPath(Guid.NewGuid().ToString()); - var test = publishTask.DownloadFileAsync( client, PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts, @@ -181,7 +180,7 @@ public void SuccessfulDownloadFileTestAsync() [InlineData(HttpStatusCode.BadRequest, 3)] [InlineData(HttpStatusCode.NotFound, 2)] [InlineData(HttpStatusCode.GatewayTimeout, 1)] - public async Task FailedDownloadTestAsync(HttpStatusCode httpStatus, int maxRetry) + public async Task DownloadFileAsyncFailsForInValidUrlTest(HttpStatusCode httpStatus, int maxRetry) { var buildEngine = new MockBuildEngine(); var publishTask = new PublishArtifactsInManifestV3 @@ -207,31 +206,65 @@ public async Task FailedDownloadTestAsync(HttpStatusCode httpStatus, int maxRetr }, new HttpResponseMessage(HttpStatusCode.NotFound) { - Content = new ByteArrayContent(responseContent) + Content = new ByteArrayContent(responseContent) } }; using HttpClient client = FakeHttpClient.WithResponses(responses); var path = TestInputs.GetFullPath(Guid.NewGuid().ToString()); - try - { - await publishTask.DownloadFileAsync( + var actualError = await Assert.ThrowsAsync(() => + publishTask.DownloadFileAsync( client, PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts, "1234", "test.txt", - path); - } - catch (Exception ex) + path)); + Assert.Contains($"Failed to download local file '{path}' after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); + } + + + [Theory] + [InlineData(HttpStatusCode.BadRequest)] + [InlineData(HttpStatusCode.NotFound)] + [InlineData(HttpStatusCode.GatewayTimeout)] + public async Task DownloadFailureWhenStatusCodeIsInvalid(HttpStatusCode httpStatus) + { + var buildEngine = new MockBuildEngine(); + var publishTask = new PublishArtifactsInManifestV3 + { + BuildEngine = buildEngine, + }; + var testFile = Path.Combine("Symbols", "test.txt"); + var responseContent = TestInputs.ReadAllBytes(testFile); + publishTask.RetryHandler = new ExponentialRetry() + { + DelayBase = 1, + MaxAttempts = 1 + }; + var responses = new[] { - Assert.Contains($"Failed to download local file '{path}' after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", ex.Message); - } + new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + } + }; + using HttpClient client = FakeHttpClient.WithResponses(responses); + var path = TestInputs.GetFullPath(Guid.NewGuid().ToString()); + + var actualError = await Assert.ThrowsAsync(() => + publishTask.DownloadFileAsync( + client, + PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts, + "1234", + "test.txt", + path)); + Assert.Contains($"Failed to download local file '{path}' after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); } [Theory] [InlineData(PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts, "1")] [InlineData(PublishArtifactsInManifestBase.ArtifactName.PackageArtifacts, "1234")] - public async Task GetContainerIdTestAsync(PublishArtifactsInManifestBase.ArtifactName artifactName, string containerId) + public async Task GetContainerIdToDownloadArtifactAsync(PublishArtifactsInManifestBase.ArtifactName artifactName, string containerId) { var buildEngine = new MockBuildEngine(); var publishTask = new PublishArtifactsInManifestV3 @@ -256,7 +289,7 @@ public async Task GetContainerIdTestAsync(PublishArtifactsInManifestBase.Artifac [Theory] [InlineData(HttpStatusCode.BadRequest, 2)] [InlineData(HttpStatusCode.NotFound, 3)] - public async Task ContainerIdErrorAsync(HttpStatusCode httpStatus, int maxAttempt) + public async Task ErrorAfterMaxRetriesToGetContainerId(HttpStatusCode httpStatus, int maxAttempt) { var buildEngine = new MockBuildEngine(); var publishTask = new PublishArtifactsInManifestV3 @@ -281,23 +314,19 @@ public async Task ContainerIdErrorAsync(HttpStatusCode httpStatus, int maxAttemp { Content = new ByteArrayContent(responseContent) }, - new HttpResponseMessage(HttpStatusCode.NotFound) + new HttpResponseMessage(httpStatus) { Content = new ByteArrayContent(responseContent) } }; using HttpClient client = FakeHttpClient.WithResponses(responses); - try - { - await publishTask.GetContainerIdAsync( + + var actualError = await Assert.ThrowsAsync(() => + publishTask.GetContainerIdAsync( client, - PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts); - } - catch (Exception ex) - { - Assert.Contains($"Failed to get container id after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", ex.Message); - } + PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts)); + Assert.Contains($"Failed to get container id after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); } } diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/TestInputs/Symbols/test.txt b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/TestInputs/Symbols/test.txt index 91eaf5a7328..5a604136aeb 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/TestInputs/Symbols/test.txt +++ b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/TestInputs/Symbols/test.txt @@ -1,35 +1,35 @@ { - "count": 1, - "value": [ - { - "id": 2124, - "name": "PackageArtifacts", - "source": "testSource", - "resource": { - "type": "Container", - "data": "#/1234/PackageArtifacts", - "properties": { - "localpath": "D:\\workspace\\_work\\1\\s\\artifacts\\output\\packages", - "artifactsize": "7644905" - }, - "url": "https://dev.azure.com/dnceng/url", - "downloadUrl": "https://dev.azure.com/dnceng/_apis/build/builds/buildId/artifacts?artifactName=PackageArtifacts&api-version=6.0&%24format=zip" - } - }, - { - "id": 2123, - "name": "BlobArtifacts", - "source": "testSource", - "resource": { - "type": "Container", - "data": "#/1/BlobArtifacts", - "properties": { - "localpath": "D:\\workspace\\_work\\1\\s\\artifacts\\output\\packages", - "artifactsize": "7644905" - }, - "url": "https://dev.azure.com/dnceng/_apis/build/builds/buildId/artifacts?artifactName=BlobArtifacts&api-version=6.0", - "downloadUrl": "https://dev.azure.com/dnceng/_apis/build/builds/buildId/artifacts?artifactName=PackageArtifacts&api-version=6.0&%24format=zip" - } - } - ] + "count":1, + "value":[ + { + "id":2124, + "name":"PackageArtifacts", + "source":"testSource", + "resource":{ + "type":"Container", + "data":"#/1234/PackageArtifacts", + "properties":{ + "localpath":"D:\\workspace\\_work\\1\\s\\artifacts\\output\\packages", + "artifactsize":"7644905" + }, + "url":"https://dev.azure.com/dnceng/url", + "downloadUrl":"https://dev.azure.com/dnceng/_apis/build/builds/buildId/artifacts?artifactName=PackageArtifacts&api-version=6.0&%24format=zip" + } + }, + { + "id":2123, + "name":"BlobArtifacts", + "source":"testSource", + "resource":{ + "type":"Container", + "data":"#/1/BlobArtifacts", + "properties":{ + "localpath":"D:\\workspace\\_work\\1\\s\\artifacts\\output\\packages", + "artifactsize":"7644905" + }, + "url":"https://dev.azure.com/dnceng/_apis/build/builds/buildId/artifacts?artifactName=BlobArtifacts&api-version=6.0", + "downloadUrl":"https://dev.azure.com/dnceng/_apis/build/builds/buildId/artifacts?artifactName=PackageArtifacts&api-version=6.0&%24format=zip" + } + } + ] } From 3cb37358fbc6fc0400eb1f1844dc999d30bc2e65 Mon Sep 17 00:00:00 2001 From: Epsitha Ananth Date: Fri, 18 Jun 2021 17:45:29 -0700 Subject: [PATCH 4/9] change response --- .../PublishToSymbolServerTest.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs index 9057a4da09d..39877c4bcd0 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs +++ b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using Microsoft.Arcade.Common; using Microsoft.Arcade.Test.Common; +using Microsoft.DotNet.Arcade.Test.Common; using Microsoft.DotNet.Build.Tasks.Feed.Model; using Microsoft.DotNet.Build.Tasks.Feed.Tests.TestDoubles; using Microsoft.DotNet.Maestro.Client.Models; @@ -161,7 +162,7 @@ public void DownloadFileAsyncSucceedsForValidUrl() Content = new ByteArrayContent(responseContent) }; - using HttpClient client = FakeHttpClient.WithResponse(response); + using HttpClient client = FakeHttpClient.WithResponses(response); var path = TestInputs.GetFullPath(Guid.NewGuid().ToString()); var test = publishTask.DownloadFileAsync( @@ -279,7 +280,7 @@ public async Task GetContainerIdToDownloadArtifactAsync(PublishArtifactsInManife Content = new ByteArrayContent(responseContent) }; - using HttpClient client = FakeHttpClient.WithResponse(responses); + using HttpClient client = FakeHttpClient.WithResponses(responses); var test = await publishTask.GetContainerIdAsync( client, artifactName); From 1d026d1bc49fb7b0061679d7aacba8f77671806c Mon Sep 17 00:00:00 2001 From: Epsitha Ananth Date: Tue, 22 Jun 2021 11:35:28 -0700 Subject: [PATCH 5/9] comments --- .../PublishToSymbolServerTest.cs | 71 ++++++++++++------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs index 39877c4bcd0..cf2a8be777d 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs +++ b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs @@ -4,11 +4,9 @@ using System.Net; using System.Net.Http; using System.Threading.Tasks; -using Microsoft.Arcade.Common; using Microsoft.Arcade.Test.Common; using Microsoft.DotNet.Arcade.Test.Common; using Microsoft.DotNet.Build.Tasks.Feed.Model; -using Microsoft.DotNet.Build.Tasks.Feed.Tests.TestDoubles; using Microsoft.DotNet.Maestro.Client.Models; using Xunit; @@ -178,10 +176,10 @@ public void DownloadFileAsyncSucceedsForValidUrl() } [Theory] - [InlineData(HttpStatusCode.BadRequest, 3)] - [InlineData(HttpStatusCode.NotFound, 2)] - [InlineData(HttpStatusCode.GatewayTimeout, 1)] - public async Task DownloadFileAsyncFailsForInValidUrlTest(HttpStatusCode httpStatus, int maxRetry) + [InlineData(HttpStatusCode.BadRequest)] + [InlineData(HttpStatusCode.NotFound)] + [InlineData(HttpStatusCode.GatewayTimeout)] + public async Task DownloadFileAsyncFailsForInValidUrlTest(HttpStatusCode httpStatus) { var buildEngine = new MockBuildEngine(); var publishTask = new PublishArtifactsInManifestV3 @@ -190,11 +188,7 @@ public async Task DownloadFileAsyncFailsForInValidUrlTest(HttpStatusCode httpSta }; var testFile = Path.Combine("Symbols", "test.txt"); var responseContent = TestInputs.ReadAllBytes(testFile); - publishTask.RetryHandler = new ExponentialRetry() - { - DelayBase = 1, - MaxAttempts = maxRetry - }; + var responses = new[] { new HttpResponseMessage(httpStatus) @@ -206,6 +200,14 @@ public async Task DownloadFileAsyncFailsForInValidUrlTest(HttpStatusCode httpSta Content = new ByteArrayContent(responseContent) }, new HttpResponseMessage(HttpStatusCode.NotFound) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(HttpStatusCode.NotFound) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(HttpStatusCode.NotFound) { Content = new ByteArrayContent(responseContent) } @@ -220,7 +222,7 @@ public async Task DownloadFileAsyncFailsForInValidUrlTest(HttpStatusCode httpSta "1234", "test.txt", path)); - Assert.Contains($"Failed to download local file '{path}' after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); + Assert.Contains($"Failed to download local file '{path}' after 5 attempts. See inner exception for details,", actualError.Message); } @@ -237,13 +239,24 @@ public async Task DownloadFailureWhenStatusCodeIsInvalid(HttpStatusCode httpStat }; var testFile = Path.Combine("Symbols", "test.txt"); var responseContent = TestInputs.ReadAllBytes(testFile); - publishTask.RetryHandler = new ExponentialRetry() - { - DelayBase = 1, - MaxAttempts = 1 - }; var responses = new[] { + new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + }, new HttpResponseMessage(httpStatus) { Content = new ByteArrayContent(responseContent) @@ -259,7 +272,7 @@ public async Task DownloadFailureWhenStatusCodeIsInvalid(HttpStatusCode httpStat "1234", "test.txt", path)); - Assert.Contains($"Failed to download local file '{path}' after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); + Assert.Contains($"Failed to download local file '{path}' after 5 attempts. See inner exception for details,", actualError.Message); } [Theory] @@ -288,9 +301,9 @@ public async Task GetContainerIdToDownloadArtifactAsync(PublishArtifactsInManife } [Theory] - [InlineData(HttpStatusCode.BadRequest, 2)] - [InlineData(HttpStatusCode.NotFound, 3)] - public async Task ErrorAfterMaxRetriesToGetContainerId(HttpStatusCode httpStatus, int maxAttempt) + [InlineData(HttpStatusCode.BadRequest)] + [InlineData(HttpStatusCode.NotFound)] + public async Task ErrorAfterMaxRetriesToGetContainerId(HttpStatusCode httpStatus) { var buildEngine = new MockBuildEngine(); var publishTask = new PublishArtifactsInManifestV3 @@ -298,11 +311,7 @@ public async Task ErrorAfterMaxRetriesToGetContainerId(HttpStatusCode httpStatus BuildEngine = buildEngine, }; publishTask.BuildId = "1243456"; - publishTask.RetryHandler = new ExponentialRetry() - { - DelayBase = 1, - MaxAttempts = maxAttempt - }; + var testPackageName = Path.Combine("Symbols", "test.txt"); var responseContent = TestInputs.ReadAllBytes(testPackageName); var responses = new[] @@ -316,6 +325,14 @@ public async Task ErrorAfterMaxRetriesToGetContainerId(HttpStatusCode httpStatus Content = new ByteArrayContent(responseContent) }, new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(httpStatus) { Content = new ByteArrayContent(responseContent) } @@ -327,7 +344,7 @@ public async Task ErrorAfterMaxRetriesToGetContainerId(HttpStatusCode httpStatus publishTask.GetContainerIdAsync( client, PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts)); - Assert.Contains($"Failed to get container id after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); + Assert.Contains($"Failed to get container id after 5 attempts. See inner exception for details,", actualError.Message); } } From 26b664101633224579c0a9cfc9909d19141a7f6b Mon Sep 17 00:00:00 2001 From: Epsitha Ananth Date: Tue, 22 Jun 2021 15:32:20 -0700 Subject: [PATCH 6/9] Revert some changes --- .../src/PublishArtifactsInManifestBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs index 8670e955c83..f2c752fca4d 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs +++ b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs @@ -185,7 +185,7 @@ public abstract class PublishArtifactsInManifestBase : Microsoft.Build.Utilities /// public int RetryDelayMilliseconds { get; set; } = 5000; - public ExponentialRetry RetryHandler = new ExponentialRetry + private readonly ExponentialRetry RetryHandler = new ExponentialRetry { MaxAttempts = 5, DelayBase = 2.5 // 2.5 ^ 5 = ~1.5 minutes max wait between retries From fafc680d01ac0314ab3bb5b67701e0e568c50508 Mon Sep 17 00:00:00 2001 From: Epsitha Ananth Date: Tue, 29 Jun 2021 17:42:16 -0700 Subject: [PATCH 7/9] changed it cos, the test were timing out --- .../PublishToSymbolServerTest.cs | 37 +++++-------------- .../src/PublishArtifactsInManifestBase.cs | 2 +- 2 files changed, 10 insertions(+), 29 deletions(-) diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs index cf2a8be777d..50cb3051cbf 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs +++ b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs @@ -4,6 +4,7 @@ using System.Net; using System.Net.Http; using System.Threading.Tasks; +using Microsoft.Arcade.Common; using Microsoft.Arcade.Test.Common; using Microsoft.DotNet.Arcade.Test.Common; using Microsoft.DotNet.Build.Tasks.Feed.Model; @@ -188,6 +189,7 @@ public async Task DownloadFileAsyncFailsForInValidUrlTest(HttpStatusCode httpSta }; var testFile = Path.Combine("Symbols", "test.txt"); var responseContent = TestInputs.ReadAllBytes(testFile); + publishTask.RetryHandler = new ExponentialRetry() { MaxAttempts = 3, DelayBase = 1 }; var responses = new[] { @@ -199,15 +201,7 @@ public async Task DownloadFileAsyncFailsForInValidUrlTest(HttpStatusCode httpSta { Content = new ByteArrayContent(responseContent) }, - new HttpResponseMessage(HttpStatusCode.NotFound) - { - Content = new ByteArrayContent(responseContent) - }, - new HttpResponseMessage(HttpStatusCode.NotFound) - { - Content = new ByteArrayContent(responseContent) - }, - new HttpResponseMessage(HttpStatusCode.NotFound) + new HttpResponseMessage(httpStatus) { Content = new ByteArrayContent(responseContent) } @@ -222,7 +216,7 @@ public async Task DownloadFileAsyncFailsForInValidUrlTest(HttpStatusCode httpSta "1234", "test.txt", path)); - Assert.Contains($"Failed to download local file '{path}' after 5 attempts. See inner exception for details,", actualError.Message); + Assert.Contains($"Failed to download local file '{path}' after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); } @@ -239,6 +233,8 @@ public async Task DownloadFailureWhenStatusCodeIsInvalid(HttpStatusCode httpStat }; var testFile = Path.Combine("Symbols", "test.txt"); var responseContent = TestInputs.ReadAllBytes(testFile); + publishTask.RetryHandler = new ExponentialRetry() { MaxAttempts = 3, DelayBase = 1 }; + var responses = new[] { new HttpResponseMessage(httpStatus) @@ -250,14 +246,6 @@ public async Task DownloadFailureWhenStatusCodeIsInvalid(HttpStatusCode httpStat Content = new ByteArrayContent(responseContent) }, new HttpResponseMessage(httpStatus) - { - Content = new ByteArrayContent(responseContent) - }, - new HttpResponseMessage(httpStatus) - { - Content = new ByteArrayContent(responseContent) - }, - new HttpResponseMessage(httpStatus) { Content = new ByteArrayContent(responseContent) } @@ -272,7 +260,7 @@ public async Task DownloadFailureWhenStatusCodeIsInvalid(HttpStatusCode httpStat "1234", "test.txt", path)); - Assert.Contains($"Failed to download local file '{path}' after 5 attempts. See inner exception for details,", actualError.Message); + Assert.Contains($"Failed to download local file '{path}' after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); } [Theory] @@ -311,6 +299,7 @@ public async Task ErrorAfterMaxRetriesToGetContainerId(HttpStatusCode httpStatus BuildEngine = buildEngine, }; publishTask.BuildId = "1243456"; + publishTask.RetryHandler = new ExponentialRetry() {MaxAttempts = 3, DelayBase = 1}; var testPackageName = Path.Combine("Symbols", "test.txt"); var responseContent = TestInputs.ReadAllBytes(testPackageName); @@ -325,14 +314,6 @@ public async Task ErrorAfterMaxRetriesToGetContainerId(HttpStatusCode httpStatus Content = new ByteArrayContent(responseContent) }, new HttpResponseMessage(httpStatus) - { - Content = new ByteArrayContent(responseContent) - }, - new HttpResponseMessage(httpStatus) - { - Content = new ByteArrayContent(responseContent) - }, - new HttpResponseMessage(httpStatus) { Content = new ByteArrayContent(responseContent) } @@ -344,7 +325,7 @@ public async Task ErrorAfterMaxRetriesToGetContainerId(HttpStatusCode httpStatus publishTask.GetContainerIdAsync( client, PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts)); - Assert.Contains($"Failed to get container id after 5 attempts. See inner exception for details,", actualError.Message); + Assert.Contains($"Failed to get container id after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); } } diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs index f2c752fca4d..8670e955c83 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs +++ b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs @@ -185,7 +185,7 @@ public abstract class PublishArtifactsInManifestBase : Microsoft.Build.Utilities /// public int RetryDelayMilliseconds { get; set; } = 5000; - private readonly ExponentialRetry RetryHandler = new ExponentialRetry + public ExponentialRetry RetryHandler = new ExponentialRetry { MaxAttempts = 5, DelayBase = 2.5 // 2.5 ^ 5 = ~1.5 minutes max wait between retries From b1296c6e56dfb85fe79446315d5327d9fc1e36f4 Mon Sep 17 00:00:00 2001 From: Epsitha Ananth Date: Wed, 30 Jun 2021 14:56:05 -0700 Subject: [PATCH 8/9] few nits --- .../PublishToSymbolServerTest.cs | 42 ++++++++++++++++++- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs index 50cb3051cbf..9e08f09b226 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs +++ b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs @@ -219,7 +219,6 @@ public async Task DownloadFileAsyncFailsForInValidUrlTest(HttpStatusCode httpSta Assert.Contains($"Failed to download local file '{path}' after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); } - [Theory] [InlineData(HttpStatusCode.BadRequest)] [InlineData(HttpStatusCode.NotFound)] @@ -263,6 +262,46 @@ public async Task DownloadFailureWhenStatusCodeIsInvalid(HttpStatusCode httpStat Assert.Contains($"Failed to download local file '{path}' after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); } + [Theory] + [InlineData(HttpStatusCode.BadRequest)] + [InlineData(HttpStatusCode.NotFound)] + [InlineData(HttpStatusCode.GatewayTimeout)] + public async Task DownloadFileSuccessfulAfterRetryTest(HttpStatusCode httpStatus) + { + var buildEngine = new MockBuildEngine(); + var publishTask = new PublishArtifactsInManifestV3 + { + BuildEngine = buildEngine, + }; + var testFile = Path.Combine("Symbols", "test.txt"); + var responseContent = TestInputs.ReadAllBytes(testFile); + publishTask.RetryHandler = new ExponentialRetry() { MaxAttempts = 2, DelayBase = 1 }; + + var responses = new[] + { + new HttpResponseMessage(httpStatus) + { + Content = new ByteArrayContent(responseContent) + }, + new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new ByteArrayContent(responseContent) + } + }; + using HttpClient client = FakeHttpClient.WithResponses(responses); + var path = TestInputs.GetFullPath(Guid.NewGuid().ToString()); + + await publishTask.DownloadFileAsync( + client, + PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts, + "1234", + "test.txt", + path); + Assert.True(File.Exists(path)); + publishTask.DeleteTemporaryFiles(path); + publishTask.DeleteTemporaryDirectory(path); + } + [Theory] [InlineData(PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts, "1")] [InlineData(PublishArtifactsInManifestBase.ArtifactName.PackageArtifacts, "1234")] @@ -327,6 +366,5 @@ public async Task ErrorAfterMaxRetriesToGetContainerId(HttpStatusCode httpStatus PublishArtifactsInManifestBase.ArtifactName.BlobArtifacts)); Assert.Contains($"Failed to get container id after {publishTask.RetryHandler.MaxAttempts} attempts. See inner exception for details,", actualError.Message); } - } } From 8fd9025be6254549a7cb24fe0ed759c344d01344 Mon Sep 17 00:00:00 2001 From: Epsitha Ananth Date: Wed, 30 Jun 2021 16:14:43 -0700 Subject: [PATCH 9/9] review comment --- .../Microsoft.DotNet.Build.Tasks.Feed.Tests.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/Microsoft.DotNet.Build.Tasks.Feed.Tests.csproj b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/Microsoft.DotNet.Build.Tasks.Feed.Tests.csproj index 528c18da1ef..548fac7ecdb 100644 --- a/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/Microsoft.DotNet.Build.Tasks.Feed.Tests.csproj +++ b/src/Microsoft.DotNet.Build.Tasks.Feed.Tests/Microsoft.DotNet.Build.Tasks.Feed.Tests.csproj @@ -39,7 +39,4 @@ - - -