mirror of
https://github.com/microsoft/semantic-kernel.git
synced 2026-10-12 02:19:06 +00:00
tests: refactors redirection tests to avoid race condition (#14342)
while working on #14340 I noticed some flaky unit tests are you can see in https://github.com/microsoft/semantic-kernel/actions/runs/32982261836/job/98221741513?pr=14340. This PR refactors the involved tests so they don't run into each other --------- Co-authored-by: Copilot Autofix powered by AI <[email protected]>
This commit is contained in:
Vincent Biret
co-authored by
Copilot Autofix powered by AI
Copilot Autofix powered by AI
parent
b4b0ae7780
commit
60264bbdc8
@@ -137,47 +137,16 @@ public sealed class HttpPluginTests : IDisposable
|
||||
public async Task ItDoesNotFollowRedirectsAsync()
|
||||
{
|
||||
// Arrange - start a local server that always returns a 302 redirect
|
||||
using var listener = new System.Net.HttpListener();
|
||||
var port = new Random().Next(49152, 65535);
|
||||
listener.Prefixes.Add($"http://localhost:{port}/");
|
||||
listener.Start();
|
||||
bool redirectTargetContacted = false;
|
||||
|
||||
_ = Task.Run(async () =>
|
||||
{
|
||||
while (listener.IsListening)
|
||||
{
|
||||
try
|
||||
{
|
||||
var ctx = await listener.GetContextAsync();
|
||||
if (ctx.Request.Url!.AbsolutePath == "/start")
|
||||
{
|
||||
ctx.Response.StatusCode = 302;
|
||||
ctx.Response.RedirectLocation = $"http://localhost:{port}/secret";
|
||||
ctx.Response.Close();
|
||||
}
|
||||
else if (ctx.Request.Url.AbsolutePath == "/secret")
|
||||
{
|
||||
redirectTargetContacted = true;
|
||||
ctx.Response.StatusCode = 200;
|
||||
ctx.Response.Close();
|
||||
}
|
||||
}
|
||||
catch (ObjectDisposedException) { break; }
|
||||
catch (System.Net.HttpListenerException) { break; }
|
||||
}
|
||||
});
|
||||
await using var server = new RedirectLoopbackServer("secret", "text/plain", []);
|
||||
|
||||
var plugin = new HttpPlugin()
|
||||
{
|
||||
AllowedDomains = ["localhost"]
|
||||
AllowedDomains = [server.BaseUri.Host]
|
||||
};
|
||||
|
||||
// Act & Assert - the plugin should throw because 302 is a non-success status
|
||||
await Assert.ThrowsAsync<HttpOperationException>(() => plugin.GetAsync($"http://localhost:{port}/start"));
|
||||
Assert.False(redirectTargetContacted, "The redirect target should not have been contacted.");
|
||||
|
||||
listener.Stop();
|
||||
await Assert.ThrowsAsync<HttpOperationException>(() => plugin.GetAsync(new Uri(server.BaseUri, "start").AbsoluteUri));
|
||||
Assert.False(server.RedirectTargetContacted, "The redirect target should not have been contacted.");
|
||||
}
|
||||
|
||||
private Mock<HttpMessageHandler> CreateMock()
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
// Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
using System;
|
||||
using System.IO;
|
||||
using System.Net;
|
||||
using System.Net.Sockets;
|
||||
using System.Text;
|
||||
using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
namespace SemanticKernel.Plugins.UnitTests;
|
||||
|
||||
internal sealed class RedirectLoopbackServer : IAsyncDisposable
|
||||
{
|
||||
private readonly TcpListener _listener;
|
||||
private readonly CancellationTokenSource _cancellationTokenSource = new();
|
||||
private readonly Task _serverTask;
|
||||
private readonly string _redirectTargetPath;
|
||||
private readonly string _targetContentType;
|
||||
private readonly byte[] _targetContent;
|
||||
private int _redirectTargetContacted;
|
||||
|
||||
public RedirectLoopbackServer(string redirectTargetPath, string targetContentType, byte[] targetContent)
|
||||
{
|
||||
this._redirectTargetPath = redirectTargetPath.StartsWith("/", StringComparison.Ordinal) ? redirectTargetPath : $"/{redirectTargetPath}";
|
||||
this._targetContentType = targetContentType;
|
||||
this._targetContent = targetContent;
|
||||
|
||||
this._listener = new TcpListener(IPAddress.Loopback, 0);
|
||||
this._listener.Start();
|
||||
var endpoint = (IPEndPoint)this._listener.LocalEndpoint;
|
||||
this.BaseUri = new Uri($"http://{endpoint.Address}:{endpoint.Port}/");
|
||||
this._serverTask = this.RunAsync();
|
||||
}
|
||||
|
||||
public Uri BaseUri { get; }
|
||||
|
||||
public bool RedirectTargetContacted => Volatile.Read(ref this._redirectTargetContacted) != 0;
|
||||
|
||||
public async ValueTask DisposeAsync()
|
||||
{
|
||||
this._cancellationTokenSource.Cancel();
|
||||
this._listener.Stop();
|
||||
await this._serverTask.ConfigureAwait(false);
|
||||
this._cancellationTokenSource.Dispose();
|
||||
}
|
||||
|
||||
private async Task RunAsync()
|
||||
{
|
||||
while (!this._cancellationTokenSource.IsCancellationRequested)
|
||||
{
|
||||
TcpClient client;
|
||||
try
|
||||
{
|
||||
client = await this._listener.AcceptTcpClientAsync(this._cancellationTokenSource.Token).ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
break;
|
||||
}
|
||||
catch (ObjectDisposedException) when (this._cancellationTokenSource.IsCancellationRequested)
|
||||
{
|
||||
break;
|
||||
}
|
||||
catch (SocketException) when (this._cancellationTokenSource.IsCancellationRequested)
|
||||
{
|
||||
break;
|
||||
}
|
||||
|
||||
using (client)
|
||||
{
|
||||
try
|
||||
{
|
||||
await this.HandleRequestAsync(client, this._cancellationTokenSource.Token).ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException) when (this._cancellationTokenSource.IsCancellationRequested)
|
||||
{
|
||||
break;
|
||||
}
|
||||
catch (ObjectDisposedException) when (this._cancellationTokenSource.IsCancellationRequested)
|
||||
{
|
||||
break;
|
||||
}
|
||||
catch (IOException)
|
||||
{
|
||||
// Ignore client disconnects / truncated requests to keep the test helper stable.
|
||||
}
|
||||
catch (SocketException) when (this._cancellationTokenSource.IsCancellationRequested)
|
||||
{
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private async Task HandleRequestAsync(TcpClient client, CancellationToken cancellationToken)
|
||||
{
|
||||
var stream = client.GetStream();
|
||||
using var reader = new StreamReader(stream, Encoding.ASCII, detectEncodingFromByteOrderMarks: false, bufferSize: 1024, leaveOpen: true);
|
||||
var requestLine = await reader.ReadLineAsync(cancellationToken).ConfigureAwait(false);
|
||||
string? header;
|
||||
do
|
||||
{
|
||||
header = await reader.ReadLineAsync(cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
while (!string.IsNullOrEmpty(header));
|
||||
|
||||
var requestTarget = requestLine?.Split(' ')[1];
|
||||
if (requestTarget == "/start")
|
||||
{
|
||||
var response = $"HTTP/1.1 302 Found\r\nLocation: {new Uri(this.BaseUri, this._redirectTargetPath.TrimStart('/'))}\r\nContent-Length: 0\r\nConnection: close\r\n\r\n";
|
||||
await stream.WriteAsync(Encoding.ASCII.GetBytes(response), cancellationToken).ConfigureAwait(false);
|
||||
return;
|
||||
}
|
||||
|
||||
Interlocked.Exchange(ref this._redirectTargetContacted, 1);
|
||||
var responseHeaders = $"HTTP/1.1 200 OK\r\nContent-Type: {this._targetContentType}\r\nContent-Length: {this._targetContent.Length}\r\nConnection: close\r\n\r\n";
|
||||
await stream.WriteAsync(Encoding.ASCII.GetBytes(responseHeaders), cancellationToken).ConfigureAwait(false);
|
||||
await stream.WriteAsync(this._targetContent, cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
@@ -82,38 +82,7 @@ public sealed class WebFileDownloadPluginTests : IDisposable
|
||||
public async Task DownloadToFileDoesNotFollowRedirectsAsync()
|
||||
{
|
||||
// Arrange - start a local server that returns a 302 redirect
|
||||
using var listener = new System.Net.HttpListener();
|
||||
var port = new Random().Next(49152, 65535);
|
||||
listener.Prefixes.Add($"http://localhost:{port}/");
|
||||
listener.Start();
|
||||
bool redirectTargetContacted = false;
|
||||
|
||||
_ = Task.Run(async () =>
|
||||
{
|
||||
while (listener.IsListening)
|
||||
{
|
||||
try
|
||||
{
|
||||
var ctx = await listener.GetContextAsync();
|
||||
if (ctx.Request.Url!.AbsolutePath == "/start")
|
||||
{
|
||||
ctx.Response.StatusCode = 302;
|
||||
ctx.Response.RedirectLocation = $"http://localhost:{port}/secret.png";
|
||||
ctx.Response.Close();
|
||||
}
|
||||
else if (ctx.Request.Url.AbsolutePath == "/secret.png")
|
||||
{
|
||||
redirectTargetContacted = true;
|
||||
ctx.Response.StatusCode = 200;
|
||||
ctx.Response.ContentType = "image/png";
|
||||
ctx.Response.OutputStream.Write(new byte[] { 0x89, 0x50, 0x4E, 0x47 });
|
||||
ctx.Response.Close();
|
||||
}
|
||||
}
|
||||
catch (ObjectDisposedException) { break; }
|
||||
catch (System.Net.HttpListenerException) { break; }
|
||||
}
|
||||
});
|
||||
await using var server = new RedirectLoopbackServer("secret.png", "image/png", [0x89, 0x50, 0x4E, 0x47]);
|
||||
|
||||
var folderPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
|
||||
var filePath = Path.Combine(folderPath, "file.png");
|
||||
@@ -121,20 +90,19 @@ public sealed class WebFileDownloadPluginTests : IDisposable
|
||||
|
||||
var webFileDownload = new WebFileDownloadPlugin()
|
||||
{
|
||||
AllowedDomains = ["localhost"],
|
||||
AllowedDomains = [server.BaseUri.Host],
|
||||
AllowedFolders = [folderPath]
|
||||
};
|
||||
|
||||
try
|
||||
{
|
||||
// Act & Assert - the plugin should throw because 302 is a non-success status
|
||||
await Assert.ThrowsAsync<HttpOperationException>(() => webFileDownload.DownloadToFileAsync(new Uri($"http://localhost:{port}/start"), filePath));
|
||||
Assert.False(redirectTargetContacted, "The redirect target should not have been contacted.");
|
||||
await Assert.ThrowsAsync<HttpOperationException>(() => webFileDownload.DownloadToFileAsync(new Uri(server.BaseUri, "start"), filePath));
|
||||
Assert.False(server.RedirectTargetContacted, "The redirect target should not have been contacted.");
|
||||
Assert.False(Path.Exists(filePath));
|
||||
}
|
||||
finally
|
||||
{
|
||||
listener.Stop();
|
||||
if (Path.Exists(folderPath))
|
||||
{
|
||||
Directory.Delete(folderPath, true);
|
||||
|
||||
Reference in New Issue
Block a user