Skip to content
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.

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

Safer formatting of Xunit assertion messages #7446

Merged
merged 5 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions src/Akka.sln
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ClusterToolsExample.Seed",
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ClusterToolsExample.Node", "examples\Cluster\ClusterTools\ClusterToolsExample.Node\ClusterToolsExample.Node.csproj", "{337A85B5-4A7C-4883-8634-46E7E52A765F}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Akka.TestKit.Xunit2.Tests", "contrib\testkits\Akka.TestKit.Xunit2.Tests\Akka.TestKit.Xunit2.Tests.csproj", "{95017C99-E960-44E5-83AD-BF21461DF06F}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -1343,6 +1345,18 @@ Global
{337A85B5-4A7C-4883-8634-46E7E52A765F}.Release|x64.Build.0 = Release|Any CPU
{337A85B5-4A7C-4883-8634-46E7E52A765F}.Release|x86.ActiveCfg = Release|Any CPU
{337A85B5-4A7C-4883-8634-46E7E52A765F}.Release|x86.Build.0 = Release|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Debug|Any CPU.Build.0 = Debug|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Debug|x64.ActiveCfg = Debug|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Debug|x64.Build.0 = Debug|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Debug|x86.ActiveCfg = Debug|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Debug|x86.Build.0 = Debug|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Release|Any CPU.ActiveCfg = Release|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Release|Any CPU.Build.0 = Release|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Release|x64.ActiveCfg = Release|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Release|x64.Build.0 = Release|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Release|x86.ActiveCfg = Release|Any CPU
{95017C99-E960-44E5-83AD-BF21461DF06F}.Release|x86.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -1469,6 +1483,7 @@ Global
{88D7D845-2F50-4D37-9026-B0A8353D0E8D} = {7735F35A-E7B7-44DE-B6FB-C770B53EB69C}
{ED00E6F4-2B5C-4F16-ADE4-45E4A73C17B8} = {7735F35A-E7B7-44DE-B6FB-C770B53EB69C}
{337A85B5-4A7C-4883-8634-46E7E52A765F} = {7735F35A-E7B7-44DE-B6FB-C770B53EB69C}
{95017C99-E960-44E5-83AD-BF21461DF06F} = {7625FD95-4B2C-4A5B-BDD5-94B1493FAC8E}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {03AD8E21-7507-4E68-A4E9-F4A7E7273164}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="../../../xunitSettings.props" />

<PropertyGroup>
<TargetFrameworks>$(NetFrameworkTestVersion);$(NetTestVersion)</TargetFrameworks>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(TestSdkVersion)" />
<PackageReference Include="xunit" Version="$(XunitVersion)" />
<PackageReference Include="xunit.runner.visualstudio" Version="$(XunitVersion)" />
<ProjectReference Include="../Akka.TestKit.Xunit2/Akka.TestKit.Xunit2.csproj" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// -----------------------------------------------------------------------
// <copyright file="AkkaEqualException.cs" company="Akka.NET Project">
// Copyright (C) 2009-2025 Lightbend Inc. <http://www.lightbend.com>
// Copyright (C) 2013-2025 .NET Foundation <https://github.com/akkadotnet/akka.net>
// </copyright>
// -----------------------------------------------------------------------

using Akka.TestKit.Xunit2.Internals;
using Xunit;

namespace Akka.TestKit.Xunit2.Tests.Internals;

public static class AkkaEqualExceptionSpec
{
#if NETFRAMEWORK
[Fact]
public static void Constructor_deserializes_message()
{
var originalException = new AkkaEqualException("Test message");

AkkaEqualException deserializedException;
using (var memoryStream = new System.IO.MemoryStream())
{
var formatter = new System.Runtime.Serialization.Formatters.Binary.BinaryFormatter();
formatter.Serialize(memoryStream, originalException);
memoryStream.Seek(0, System.IO.SeekOrigin.Begin);
deserializedException = (AkkaEqualException)formatter.Deserialize(memoryStream);
}

Assert.Equal(originalException.Message, deserializedException.Message);
}
#endif
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// -----------------------------------------------------------------------
// <copyright file="XunitAssertionsTests.cs" company="Akka.NET Project">
// Copyright (C) 2009-2025 Lightbend Inc. <http://www.lightbend.com>
// Copyright (C) 2013-2025 .NET Foundation <https://github.com/akkadotnet/akka.net>
// </copyright>
// -----------------------------------------------------------------------

using Xunit;
using Xunit.Sdk;

namespace Akka.TestKit.Xunit2.Tests;

public class XunitAssertionsSpec
{
private readonly XunitAssertions _assertions = new();

[Fact]
public void Assert_does_not_format_message_when_no_arguments_are_specified()
{
const string testMessage = "{Value} with different format placeholders {0}";

var exception = Assert.ThrowsAny<XunitException>(() => _assertions.Fail(testMessage));
Assert.Contains(testMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertTrue(false, testMessage));
Assert.Contains(testMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertFalse(true, testMessage));
Assert.Contains(testMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertEqual(4, 2, testMessage));
Assert.Contains(testMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertEqual(4, 2, (_, _) => false, testMessage));
Assert.Contains(testMessage, exception.Message);
}

[Fact]
public void Assert_formats_message_when_arguments_are_specified()
{
const string testMessage = "Meaning: {0}";
const string expectedMessage = "Meaning: 42";

var exception = Assert.ThrowsAny<XunitException>(() => _assertions.Fail(testMessage, 42));
Assert.Contains(expectedMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertTrue(false, testMessage, 42));
Assert.Contains(expectedMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertFalse(true, testMessage, 42));
Assert.Contains(expectedMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertEqual(4, 2, testMessage, 42));
Assert.Contains(expectedMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertEqual(4, 2, (_, _) => false, testMessage, 42));
Assert.Contains(expectedMessage, exception.Message);
}

[Fact]
public void Assert_catches_format_exceptions()
{
const string testMessage = "Meaning: {0} {1}";
const string expectedMessage = "Could not string.Format";

var exception = Assert.ThrowsAny<XunitException>(() => _assertions.Fail(testMessage, 42));
Assert.Contains(expectedMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertTrue(false, testMessage, 42));
Assert.Contains(expectedMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertFalse(true, testMessage, 42));
Assert.Contains(expectedMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertEqual(4, 2, testMessage, 42));
Assert.Contains(expectedMessage, exception.Message);

exception = Assert.ThrowsAny<XunitException>(() => _assertions.AssertEqual(4, 2, (_, _) => false, testMessage, 42));
Assert.Contains(expectedMessage, exception.Message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,12 @@ namespace Akka.TestKit.Xunit2.Internals
/// <summary>
/// TBD
/// </summary>
[Serializable]
public class AkkaEqualException : XunitException
{
// Length of "Expected: " and "Actual: "
private static readonly string NewLineAndIndent = Environment.NewLine + new string(' ', 10);

private readonly string? _format;
private readonly object[] _args = Array.Empty<object>();

public static AkkaEqualException ForMismatchedValues(
object? expected,
object? actual,
Expand All @@ -46,48 +44,46 @@ public static AkkaEqualException ForMismatchedValues(
args
);
}

/// <summary>
/// Initializes a new instance of the <see cref="AkkaEqualException"/> class.
/// </summary>
/// <param name="format">A template string that describes the error.</param>
/// <param name="args">An optional object array that contains zero or more objects to format.</param>
public AkkaEqualException(string format = "", params object[] args): base(null)
{
_format = format;
_args = args;
}
public AkkaEqualException(string format = "", params object[] args)
: base(BuildAssertionMessage(format, args)) { }

/// <summary>
/// Initializes a new instance of the <see cref="AkkaEqualException"/> class.
/// </summary>
/// <param name="info">The <see cref="SerializationInfo"/> that holds the serialized object data about the exception being thrown.</param>
/// <param name="context">The <see cref="StreamingContext"/> that contains contextual information about the source or destination.</param>
protected AkkaEqualException(SerializationInfo info, StreamingContext context): base(null)
{
}
protected AkkaEqualException(SerializationInfo info, StreamingContext context)
: base(info.GetString("Message")) { }

/// <summary>
/// The message that describes the error.
/// Builds assertion message by applying specified arguments to the format string.
/// When no arguments are specified, format string is returned as-is.
/// </summary>
public override string Message
internal static string? BuildAssertionMessage(string format, object[] args)
{
get
if (string.IsNullOrEmpty(format))
{
if(string.IsNullOrEmpty(_format))
return base.Message;
return null;
}

string message;
try
{
message = string.Format(_format!, _args);
}
catch(Exception)
{
message = $@"[Could not string.Format(""{_format}"", {string.Join(", ", _args)})]";
}
if (args is not { Length: > 0 })
{
return format;
}

return base.Message is not null ? $"{base.Message} {message}" : message;
try
{
return string.Format(format, args);
}
catch (Exception)
{
return $"""[Could not string.Format("{format}", {string.Join(", ", args)})]""";
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/contrib/testkits/Akka.TestKit.Xunit2/XunitAssertions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class XunitAssertions : ITestKitAssertions
/// <param name="args">An optional object array that contains zero or more objects to format.</param>
public void Fail(string format = "", params object[] args)
{
Assert.Fail(string.Format(format, args));
Assert.Fail(AkkaEqualException.BuildAssertionMessage(format, args));
}

/// <summary>
Expand All @@ -34,7 +34,7 @@ public void Fail(string format = "", params object[] args)
/// <param name="args">An optional object array that contains zero or more objects to format.</param>
public void AssertTrue(bool condition, string format = "", params object[] args)
{
Assert.True(condition, string.Format(format, args));
Assert.True(condition, AkkaEqualException.BuildAssertionMessage(format, args));
}

/// <summary>
Expand All @@ -45,7 +45,7 @@ public void AssertTrue(bool condition, string format = "", params object[] args)
/// <param name="args">An optional object array that contains zero or more objects to format.</param>
public void AssertFalse(bool condition, string format = "", params object[] args)
{
Assert.False(condition, string.Format(format, args));
Assert.False(condition, AkkaEqualException.BuildAssertionMessage(format, args));
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ namespace Akka.TestKit.Xunit2.Internals
{
public AkkaEqualException(string format = "", params object[] args) { }
protected AkkaEqualException(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { }
public override string Message { get; }
[return: System.Runtime.CompilerServices.NullableAttribute(1)]
public static Akka.TestKit.Xunit2.Internals.AkkaEqualException ForMismatchedValues(object expected, object actual, string format = null, [System.Runtime.CompilerServices.NullableAttribute(1)] params object[] args) { }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ namespace Akka.TestKit.Xunit2.Internals
{
public AkkaEqualException(string format = "", params object[] args) { }
protected AkkaEqualException(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { }
public override string Message { get; }
[return: System.Runtime.CompilerServices.NullableAttribute(1)]
public static Akka.TestKit.Xunit2.Internals.AkkaEqualException ForMismatchedValues(object expected, object actual, string format = null, [System.Runtime.CompilerServices.NullableAttribute(1)] params object[] args) { }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public async Task cancelable_delay_must_call_next_interceptor_immediately_after_

var startedAt = DateTime.Now;
var task = delay.InterceptAsync(null);
await Task.Delay(delayDuration - epsilon);
await Task.Delay(delayDuration);

probe.WasCalled.Should().BeFalse();
cts.Cancel();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public async Task cancelable_delay_must_call_next_interceptor_immediately_after_

var startedAt = DateTime.Now;
var task = delay.InterceptAsync(null, null);
await Task.Delay(delayDuration - epsilon);
await Task.Delay(delayDuration);

probe.WasCalled.Should().BeFalse();
cts.Cancel();
Expand Down
Loading