Skip to content

Commit

Permalink
Safer formatting of Xunit assertion messages (#7446)
Browse files Browse the repository at this point in the history
* Simplify "AkkaEqualException" + format message only when needed

* Use safe message formatting in all "XunitAssertions" methods

* Add tests for "Akka.TestKit.Xunit2"

* Update expected API shape for Xunit2

* Do not shorten delay by epsilon (test failures)
  • Loading branch information
milang authored Jan 8, 2025
1 parent 848cd4c commit 26383ec
Show file tree
Hide file tree
Showing 10 changed files with 173 additions and 34 deletions.
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

0 comments on commit 26383ec

Please sign in to comment.