Skip to content

Commit

Permalink
Removed irrelevant stack frames from exception stack traces (shouldl…
Browse files Browse the repository at this point in the history
…y#501)

* Failing tests for relevant stack trace

* Removed irrelevant stack frames from exceptions

* Failing tests for matching trailing whitespace of default stack trace

* Match trailing whitespace of default exception stack trace
  • Loading branch information
jnm2 authored and josephwoodward committed Sep 21, 2018
1 parent 86e42dd commit b3acc89
Show file tree
Hide file tree
Showing 7 changed files with 271 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ public class ShouldAssertException : Shouldly.ChuckedAWobbly
{
public ShouldAssertException(string message) { }
public ShouldAssertException(string message, System.Exception innerException) { }
public override string StackTrace { get; }
}
[Shouldly.ShouldlyMethodsAttribute()]
public class static ShouldBeBooleanExtensions
Expand Down Expand Up @@ -489,6 +490,7 @@ public class ShouldlyTimeoutException : System.TimeoutException
{
public ShouldlyTimeoutException() { }
public ShouldlyTimeoutException(string message, Shouldly.ShouldlyTimeoutException inner) { }
public override string StackTrace { get; }
}
public class ShouldMatchApprovedException : Shouldly.ShouldAssertException
{
Expand Down
41 changes: 41 additions & 0 deletions src/Shouldly.Tests/StackTraceTests.ExceptionThrower.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
using System;

namespace Shouldly.Tests
{
partial class StackTraceTests
{
public sealed class ExceptionThrower
{
public Type ExceptionType { get; }
public bool InShouldlyAssembly { get; }
public Action ThrowingAction { get; }

public ExceptionThrower(Type exceptionType, bool inShouldlyAssembly, Action throwingAction)
{
ExceptionType = exceptionType ?? throw new ArgumentNullException(nameof(exceptionType));
InShouldlyAssembly = inShouldlyAssembly;
ThrowingAction = throwingAction ?? throw new ArgumentNullException(nameof(throwingAction));
}

public override string ToString()
{
return InShouldlyAssembly ? ThrowingAction.Method.Name :
ExceptionType.Name + " thrown directly";
}

public Exception Catch()
{
// Don’t rely on a framework for this in case of the outside chance that the framework manipulates the stack trace.
try
{
ThrowingAction.Invoke();
return null;
}
catch (Exception ex) when (ex.GetType() == ExceptionType)
{
return ex;
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
using System;
using System.Collections.Generic;
using System.Linq;

namespace Shouldly.Tests
{
partial class StackTraceTests
{
private sealed class ExceptionThrowerCollectionBuilder
{
private readonly List<ExceptionThrower> exceptionThrowers = new List<ExceptionThrower>();

/// <param name="throwDirectly">Required to cover the code path where the stack trace is not trimmed.</param>
/// <param name="throwInShouldlyAssembly">Required to cover the code path where the stack trace is trimmed.</param>
public ExceptionThrowerCollectionBuilder Add<TException>(Action throwDirectly, params Action[] throwInShouldlyAssembly) where TException : Exception
{
exceptionThrowers.Add(new ExceptionThrower(typeof(TException), false, throwDirectly));

if (throwInShouldlyAssembly.Length == 0)
throw new ArgumentException("If an action cannot be provided which throws this type of exception in the Shouldly assembly, specify the reason in the other overload.", nameof(throwInShouldlyAssembly));

foreach (var action in throwInShouldlyAssembly)
exceptionThrowers.Add(new ExceptionThrower(typeof(TException), true, action));

return this;
}

/// <param name="throwDirectly">Required to cover the code path where the stack trace is not trimmed.</param>
/// <param name="reasonNotThrowingFromShouldlyAssembly">
/// Reason that an action cannot be provided which throws this type of exception in the Shouldly assembly.
/// </param>
public ExceptionThrowerCollectionBuilder Add<TException>(Action throwDirectly, string reasonNotThrowingFromShouldlyAssembly) where TException : Exception
{
exceptionThrowers.Add(new ExceptionThrower(typeof(TException), false, throwDirectly));
return this;
}

public IReadOnlyCollection<ExceptionThrower> Build()
{
var missingExceptionTypes = new List<string>();

foreach (var type in typeof(ShouldAssertException).Assembly.GetExportedTypes())
{
if (type.IsSubclassOf(typeof(Exception)))
{
if (exceptionThrowers.All(t => t.ExceptionType != type))
{
missingExceptionTypes.Add(type.Name);
}
}
}

if (missingExceptionTypes.Any())
{
throw new InvalidOperationException("All exception types must be covered by these tests. Missing: " + string.Join(", ", missingExceptionTypes));
}

return exceptionThrowers;
}
}
}
}
90 changes: 90 additions & 0 deletions src/Shouldly.Tests/StackTraceTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Xunit;

namespace Shouldly.Tests
{
public static partial class StackTraceTests
{
[Theory]
[MemberData(nameof(ExceptionThrowers))]
public static void Top_stack_frame_is_user_code(ExceptionThrower exceptionThrower)
{
var exception = exceptionThrower.Catch();

var stackTraceLines = exception.StackTrace.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries);

stackTraceLines.First().ShouldContain(exceptionThrower.ThrowingAction.Method.Name);
}

[Theory]
[MemberData(nameof(ExceptionThrowers))]
public static void Stack_trace_is_trimmed_the_same_as_default_exception_stack_traces(ExceptionThrower exceptionThrower)
{
var shouldlyException = exceptionThrower.Catch();
var defaultException = new ExceptionThrower(typeof(Exception), false, () => throw new Exception()).Catch();

var shouldlyEndingWhitespace = GetEndingWhitespace(shouldlyException.StackTrace);
var defaultEndingWhitespace = GetEndingWhitespace(defaultException.StackTrace);

shouldlyEndingWhitespace.ShouldBe(defaultEndingWhitespace);
}

private static string GetEndingWhitespace(string value)
{
return value.Substring(value.TrimEnd().Length);
}

public static IEnumerable<object[]> ExceptionThrowers()
{
return new ExceptionThrowerCollectionBuilder()
#pragma warning disable 618
.Add<ChuckedAWobbly>(
throwDirectly: () => throw new ChuckedAWobbly(null),
reasonNotThrowingFromShouldlyAssembly: "Exact type not thrown in Shouldly assembly")
#pragma warning restore 618

.Add<ShouldAssertException>(
throwDirectly: () => throw new ShouldAssertException(null),
throwInShouldlyAssembly: new Action[]
{
FailingUserCode_ShouldBeTrue,
FailingUserCode_ShouldContain
})

.Add<ShouldlyTimeoutException>(
throwDirectly: () => throw new ShouldlyTimeoutException(null, null),
reasonNotThrowingFromShouldlyAssembly: "Exact type not thrown in Shouldly assembly")

.Add<ShouldCompleteInException>(
throwDirectly: () => throw new ShouldCompleteInException(null, null),
throwInShouldlyAssembly: FailingUserCode_CompleteIn)

.Add<ShouldMatchApprovedException>(
throwDirectly: () => throw new ShouldMatchApprovedException(null, null, null),
reasonNotThrowingFromShouldlyAssembly: "Don’t want to actually create a file on disk")

.Build()
.Select(exceptionThrower => new object[] { exceptionThrower });
}

private static void FailingUserCode_ShouldBeTrue()
{
false.ShouldBeTrue();
}

private static void FailingUserCode_ShouldContain()
{
// Causes a few more frames that need to be filtered
"".ShouldContain("42");
}

private static void FailingUserCode_CompleteIn()
{
// Throws a different exception type
Should.CompleteIn(Task.Delay(15), TimeSpan.Zero);
}
}
}
61 changes: 61 additions & 0 deletions src/Shouldly/Internals/StackTraceHelpers.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#if StackTrace
using System;
using System.Diagnostics;
using System.Reflection;
using System.Text;

namespace Shouldly.Internals
{
internal static class StackTraceHelpers
{
public static string GetStackTrace(Exception exception, ref string cachedValue)
{
if (cachedValue == null)
{
var builder = new StringBuilder();
WriteFilteredStackTrace(builder, new StackTrace(exception, fNeedFileInfo: true));
cachedValue = builder.ToString();
}

return cachedValue;
}

public static void WriteFilteredStackTrace(StringBuilder builder, StackTrace stackTrace)
{
var shouldlyAssembly = Assembly.GetExecutingAssembly();

for (var startIndex = 0; startIndex < stackTrace.FrameCount; startIndex++)
{
var frame = stackTrace.GetFrame(startIndex);
if (frame.GetMethod().DeclaringType?.Assembly != shouldlyAssembly)
{
if (startIndex == 0)
{
builder.Append(stackTrace.ToString().TrimEnd());
}
else
{
var lines = new string[stackTrace.FrameCount - startIndex];
var neededCapacity = builder.Length;

for (var i = 0; i < lines.Length; i++)
{
var line = new StackTrace(stackTrace.GetFrame(i + startIndex)).ToString();
if (i == lines.Length - 1) line = line.TrimEnd();
lines[i] = line;
neededCapacity += line.Length;
}

builder.EnsureCapacity(neededCapacity);

foreach (var line in lines)
builder.Append(line);
}

return;
}
}
}
}
}
#endif
9 changes: 8 additions & 1 deletion src/Shouldly/ShouldAssertException.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using Shouldly.Internals;

namespace Shouldly
{
Expand All @@ -16,5 +17,11 @@ public ShouldAssertException(string message) : base(message)
public ShouldAssertException(string message, Exception innerException) : base(message, innerException)
{
}

#if StackTrace
private string stackTrace;

public override string StackTrace => StackTraceHelpers.GetStackTrace(this, ref stackTrace);
#endif
}
}
}
7 changes: 7 additions & 0 deletions src/Shouldly/ShouldlyTimeoutException.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using Shouldly.Internals;

namespace Shouldly
{
Expand All @@ -11,5 +12,11 @@ public ShouldlyTimeoutException() : base()
public ShouldlyTimeoutException(string message, ShouldlyTimeoutException inner) : base(message, inner)
{
}

#if StackTrace
private string stackTrace;

public override string StackTrace => StackTraceHelpers.GetStackTrace(this, ref stackTrace);
#endif
}
}

0 comments on commit b3acc89

Please sign in to comment.