Skip to content
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
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using System;
using System.IO;
using System.Threading.Tasks;
using Cuemon;
using Cuemon.Collections.Generic;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Hosting.StaticWebAssets;
Expand Down Expand Up @@ -40,8 +38,8 @@ public AspNetCoreHostFixture()
/// </exception>
public override void ConfigureHost(Test hostTest)
{
Validator.ThrowIfNull(hostTest);
Validator.ThrowIfNotContainsType(hostTest, Arguments.ToArrayOf(typeof(AspNetCoreHostTest<>)), $"{nameof(hostTest)} is not assignable from AspNetCoreHostTest<T>.");
if (hostTest == null) { throw new ArgumentNullException(nameof(hostTest)); }
if (!HasTypes(hostTest.GetType(), typeof(HostTest<>))) { throw new ArgumentOutOfRangeException(nameof(hostTest), typeof(HostTest<>), $"{nameof(hostTest)} is not assignable from AspNetCoreHostTest<T>."); }

var hb = new HostBuilder()
.ConfigureWebHost(webBuilder =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@
<PackageReference Include="Microsoft.AspNetCore.TestHost" Version="8.0.8" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="Cuemon.Core" Version="9.0.0-preview.9" PrivateAssets="compile" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\Codebelt.Extensions.Xunit.Hosting\Codebelt.Extensions.Xunit.Hosting.csproj" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using System.IO;
using System.Text;
using Codebelt.Extensions.Xunit.Hosting.AspNetCore.Http.Features;
using Cuemon;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;

Expand All @@ -29,15 +28,33 @@ public FakeHttpContextAccessor()

private Stream MakeGreeting(string greeting)
{
return Patterns.SafeInvoke(() => new MemoryStream(), ms =>
Stream interim = null;
Stream result = null;
try
{
var sw = new StreamWriter(ms, Encoding.UTF8);
sw.Write(greeting);
sw.Flush();
ms.Flush();
ms.Position = 0;
return ms;
}, ex => throw new InvalidOperationException("There is an error in the Stream being written.", ex));
interim = new MemoryStream();

using (var sw = new StreamWriter(interim, Encoding.UTF8, leaveOpen: true))
{
sw.Write(greeting);
sw.Flush();
}

interim.Flush();
interim.Position = 0;

result = interim;
interim = null;
}
catch (Exception ex)
{
throw new InvalidOperationException("There is an error in the Stream being written.", ex);
}
Comment on lines +49 to +52
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid catching general Exception; catch specific exceptions instead

Catching the general Exception can mask unexpected errors and make debugging more difficult. Consider catching specific exceptions that might occur in this context, such as IOException, to handle anticipated issues more precisely.

finally
{
interim?.Dispose();
}
return result;
Comment on lines +31 to +57
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider simplifying the MakeGreeting method to improve clarity and resource management

The current implementation of the MakeGreeting method uses multiple variables and includes a try-catch-finally block for resource management. This can be simplified by utilizing using statements, which automatically handle resource disposal, and allowing exceptions to propagate naturally. This approach enhances readability and maintainability.

Here's a proposed refactoring:

-private Stream MakeGreeting(string greeting)
-{
-    Stream interim = null;
-    Stream result = null;
-    try
-    {
-        interim = new MemoryStream();
-        using (var sw = new StreamWriter(interim, Encoding.UTF8, leaveOpen: true))
-        {
-            sw.Write(greeting);
-            sw.Flush();
-        }
-        interim.Flush();
-        interim.Position = 0;
-        result = interim;
-        interim = null;
-    }
-    catch (Exception ex)
-    {
-        throw new InvalidOperationException("There is an error in the Stream being written.", ex);
-    }
-    finally
-    {
-        interim?.Dispose();
-    }
-    return result;
-}
+private Stream MakeGreeting(string greeting)
+{
+    var memoryStream = new MemoryStream();
+    using (var writer = new StreamWriter(memoryStream, Encoding.UTF8, leaveOpen: true))
+    {
+        writer.Write(greeting);
+        writer.Flush();
+    }
+    memoryStream.Position = 0;
+    return memoryStream;
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Stream interim = null;
Stream result = null;
try
{
var sw = new StreamWriter(ms, Encoding.UTF8);
sw.Write(greeting);
sw.Flush();
ms.Flush();
ms.Position = 0;
return ms;
}, ex => throw new InvalidOperationException("There is an error in the Stream being written.", ex));
interim = new MemoryStream();
using (var sw = new StreamWriter(interim, Encoding.UTF8, leaveOpen: true))
{
sw.Write(greeting);
sw.Flush();
}
interim.Flush();
interim.Position = 0;
result = interim;
interim = null;
}
catch (Exception ex)
{
throw new InvalidOperationException("There is an error in the Stream being written.", ex);
}
finally
{
interim?.Dispose();
}
return result;
private Stream MakeGreeting(string greeting)
{
var memoryStream = new MemoryStream();
using (var writer = new StreamWriter(memoryStream, Encoding.UTF8, leaveOpen: true))
{
writer.Write(greeting);
writer.Flush();
}
memoryStream.Position = 0;
return memoryStream;
}

}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Net.Http;
using System.Threading.Tasks;
using Cuemon;

namespace Codebelt.Extensions.Xunit.Hosting.AspNetCore
{
Expand All @@ -22,7 +21,7 @@ public static class HttpClientExtensions
/// <remarks>Designed to be used in conjunction with <see cref="WebHostTestFactory.RunAsync"/> and <see cref="WebHostTestFactory.RunWithHostBuilderContextAsync"/>.</remarks>
public static async Task<HttpResponseMessage> ToHttpResponseMessageAsync(this HttpClient client, Func<HttpClient, Task<HttpResponseMessage>> responseFactory = null)
{
Validator.ThrowIfNull(client);
if (client == null) { throw new ArgumentNullException(nameof(client)); }
responseFactory ??= c => c.GetAsync("/");
return await responseFactory(client).ConfigureAwait(false);
}
Expand Down
12 changes: 12 additions & 0 deletions src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/Tweaker.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using System;

namespace Codebelt.Extensions.Xunit.Hosting.AspNetCore
{
internal static class Tweaker
{
internal static T Adjust<T>(T value, Func<T, T> converter)
{
return converter == null ? value : converter.Invoke(value);
}
}
}
Comment on lines +1 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider a more descriptive class name.

The current implementation of the Tweaker class is simple, flexible, and serves its purpose well. However, the class name "Tweaker" might be a bit vague and doesn't immediately convey its specific functionality.

Consider renaming the class to something more descriptive of its purpose, such as ValueAdjuster or ConditionalConverter. This would make the class's role more immediately apparent to other developers. For example:

internal static class ValueAdjuster
{
    internal static T Adjust<T>(T value, Func<T, T> converter)
    {
        return converter == null ? value : converter.Invoke(value);
    }
}

However, if "Tweaker" is a term commonly used in your project or team's vocabulary for this kind of operation, feel free to keep it as is.

Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using Cuemon;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Cuemon.Core" Version="9.0.0-preview.9" PrivateAssets="compile" />
<PackageReference Include="xunit.extensibility.core" Version="2.9.2" />
</ItemGroup>

Expand Down
1 change: 0 additions & 1 deletion src/Codebelt.Extensions.Xunit.Hosting/GenericHostTest.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using Cuemon;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;

Expand Down
27 changes: 23 additions & 4 deletions src/Codebelt.Extensions.Xunit.Hosting/HostFixture.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
using System;
using System.IO;
using Cuemon;
using Cuemon.Collections.Generic;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
Expand Down Expand Up @@ -34,8 +32,8 @@ public HostFixture()
/// </exception>
public virtual void ConfigureHost(Test hostTest)
{
Validator.ThrowIfNull(hostTest);
Validator.ThrowIfNotContainsType(hostTest, Arguments.ToArrayOf(typeof(HostTest<>)), $"{nameof(hostTest)} is not assignable from HostTest<T>.");
if (hostTest == null) { throw new ArgumentNullException(nameof(hostTest)); }
if (!HasTypes(hostTest.GetType(), typeof(HostTest<>))) { throw new ArgumentOutOfRangeException(nameof(hostTest), typeof(HostTest<>), $"{nameof(hostTest)} is not assignable from HostTest<T>."); }

var hb = new HostBuilder()
.UseContentRoot(Directory.GetCurrentDirectory())
Expand Down Expand Up @@ -65,6 +63,27 @@ public virtual void ConfigureHost(Test hostTest)
Host = hb.Build();
}

/// <summary>
/// Determines whether the specified <paramref name="type"/> contains one or more of the specified target <paramref name="types"/>.
/// </summary>
/// <param name="type">The <see cref="Type"/> to validate.</param>
/// <param name="types">The target types to be matched against.</param>
/// <returns><c>true</c> if the <paramref name="type"/> contains one or more of the specified target types; otherwise, <c>false</c>.</returns>
protected static bool HasTypes(Type type, params Type[] types)
{
foreach (var tt in types)
{
var st = type;
while (st != null)
{
if (st.IsGenericType && tt == st.GetGenericTypeDefinition()) { return true; }
if (st == tt) { return true; }
st = st.BaseType;
}
}
return false;
}

#if NETSTANDARD2_0_OR_GREATER
/// <summary>
/// Gets or sets the delegate that initializes the test class.
Expand Down
3 changes: 1 addition & 2 deletions src/Codebelt.Extensions.Xunit.Hosting/HostTest.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using Cuemon;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
Expand All @@ -25,7 +24,7 @@ public abstract class HostTest<T> : Test, IClassFixture<T> where T : class, IHos
/// <param name="callerType">The <see cref="Type"/> of caller that ends up invoking this instance.</param>
protected HostTest(T hostFixture, ITestOutputHelper output = null, Type callerType = null) : base(output, callerType)
{
Validator.ThrowIfNull(hostFixture);
if (hostFixture == null) { throw new ArgumentNullException(nameof(hostFixture)); }
InitializeHostFixture(hostFixture);
}

Expand Down
8 changes: 4 additions & 4 deletions src/Codebelt.Extensions.Xunit.Hosting/LoggerExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using System;
using System.Collections;
using System.Linq;
using Cuemon;
using System.Reflection;
using Microsoft.Extensions.Logging;

namespace Codebelt.Extensions.Xunit.Hosting
Expand All @@ -24,13 +24,13 @@ public static class LoggerExtensions
/// </exception>
public static ITestStore<XunitTestLoggerEntry> GetTestStore<T>(this ILogger<T> logger)
{
Validator.ThrowIfNull(logger);
if (logger == null) { throw new ArgumentNullException(nameof(logger)); }
var loggerType = logger.GetType();
var internalLogger = Decorator.Enclose(loggerType).GetAllFields().SingleOrDefault(fi => fi.Name == "_logger")?.GetValue(logger);
var internalLogger = loggerType.GetRuntimeFields().SingleOrDefault(fi => fi.Name == "_logger")?.GetValue(logger);
if (internalLogger != null)
{
var internalLoggerType = internalLogger.GetType();
var internalLoggers = Decorator.Enclose(internalLoggerType).GetAllProperties().SingleOrDefault(pi => pi.Name == "Loggers")?.GetValue(internalLogger);
var internalLoggers = internalLoggerType.GetRuntimeProperties().SingleOrDefault(pi => pi.Name == "Loggers")?.GetValue(internalLogger);
if (internalLoggers != null)
{
foreach (var loggerInformation in (IEnumerable)internalLoggers)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System;
using System.Linq;
using Cuemon;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Xunit.Abstractions;
Expand All @@ -25,8 +24,8 @@ public static class ServiceCollectionExtensions
/// </exception>
public static IServiceCollection AddXunitTestLogging(this IServiceCollection services, ITestOutputHelper output, LogLevel minimumLevel = LogLevel.Trace)
{
Validator.ThrowIfNull(services);
Validator.ThrowIfNull(output);
if (services == null) { throw new ArgumentNullException(nameof(services)); }
if (output == null) { throw new ArgumentNullException(nameof(output)); }
if (services.Any(sd => sd.ServiceType == typeof(ITestOutputHelperAccessor)))
{
services.AddLogging(builder =>
Expand Down Expand Up @@ -73,7 +72,7 @@ public static IServiceCollection AddXunitTestLoggingOutputHelperAccessor(this IS
/// </exception>
public static IServiceCollection AddXunitTestLoggingOutputHelperAccessor<T>(this IServiceCollection services) where T : class, ITestOutputHelperAccessor
{
Validator.ThrowIfNull(services);
if (services == null) { throw new ArgumentNullException(nameof(services)); }
services.AddSingleton<ITestOutputHelperAccessor, T>();
return services;
}
Expand Down
12 changes: 12 additions & 0 deletions src/Codebelt.Extensions.Xunit.Hosting/Tweaker.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using System;

namespace Codebelt.Extensions.Xunit.Hosting
{
internal static class Tweaker
{
internal static T Adjust<T>(T value, Func<T, T> converter)
{
return converter == null ? value : converter.Invoke(value);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Cuemon.Core" Version="9.0.0-preview.9" PrivateAssets="compile" />

<PackageReference Include="xunit.assert" Version="2.9.2" />
<PackageReference Include="xunit.abstractions" Version="2.0.3" />
</ItemGroup>
Expand Down
27 changes: 27 additions & 0 deletions src/Codebelt.Extensions.Xunit/DelimitedString.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using System;
using System.Collections.Generic;
using System.Text;

namespace Codebelt.Extensions.Xunit
{
internal static class DelimitedString
{
internal static string Create<T>(IEnumerable<T> source, Action<DelimitedStringOptions<T>> setup = null)
{
if (source == null) { throw new ArgumentNullException(nameof(source)); }

var options = new DelimitedStringOptions<T>();
setup?.Invoke(options);

var delimitedValues = new StringBuilder();
using (var enumerator = source.GetEnumerator())
{
while (enumerator.MoveNext())
{
delimitedValues.Append(FormattableString.Invariant($"{options.StringConverter(enumerator.Current)}{options.Delimiter}"));
}
}
Comment on lines +13 to +23
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Optimize for empty source collections.

The current implementation always allocates a StringBuilder, even when the source collection is empty. This can be optimized to avoid unnecessary allocation.

Consider adding a check for an empty source before allocating the StringBuilder:

 var options = new DelimitedStringOptions<T>();
 setup?.Invoke(options);

+ if (!source.Any())
+ {
+     return string.Empty;
+ }
+
 var delimitedValues = new StringBuilder();
 using (var enumerator = source.GetEnumerator())
 {
     while (enumerator.MoveNext())
     {
         delimitedValues.Append(FormattableString.Invariant($"{options.StringConverter(enumerator.Current)}{options.Delimiter}"));
     }
 }

This change will return an empty string immediately if the source is empty, avoiding unnecessary StringBuilder allocation and improving performance for empty collections.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var options = new DelimitedStringOptions<T>();
setup?.Invoke(options);
var delimitedValues = new StringBuilder();
using (var enumerator = source.GetEnumerator())
{
while (enumerator.MoveNext())
{
delimitedValues.Append(FormattableString.Invariant($"{options.StringConverter(enumerator.Current)}{options.Delimiter}"));
}
}
var options = new DelimitedStringOptions<T>();
setup?.Invoke(options);
if (!source.Any())
{
return string.Empty;
}
var delimitedValues = new StringBuilder();
using (var enumerator = source.GetEnumerator())
{
while (enumerator.MoveNext())
{
delimitedValues.Append(FormattableString.Invariant($"{options.StringConverter(enumerator.Current)}{options.Delimiter}"));
}
}

return delimitedValues.Length > 0 ? delimitedValues.ToString(0, delimitedValues.Length - options.Delimiter.Length) : delimitedValues.ToString();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor for improved readability.

The current return statement is correct but could be made more readable by breaking it into multiple lines or using a more explicit if-else structure.

Consider refactoring the return statement for improved readability:

- return delimitedValues.Length > 0 ? delimitedValues.ToString(0, delimitedValues.Length - options.Delimiter.Length) : delimitedValues.ToString();
+ if (delimitedValues.Length > 0)
+ {
+     return delimitedValues.ToString(0, delimitedValues.Length - options.Delimiter.Length);
+ }
+ return string.Empty;

This change makes the logic more explicit and easier to read, while maintaining the same functionality.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return delimitedValues.Length > 0 ? delimitedValues.ToString(0, delimitedValues.Length - options.Delimiter.Length) : delimitedValues.ToString();
if (delimitedValues.Length > 0)
{
return delimitedValues.ToString(0, delimitedValues.Length - options.Delimiter.Length);
}
return string.Empty;

}
}
}
17 changes: 17 additions & 0 deletions src/Codebelt.Extensions.Xunit/DelimitedStringOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System;

namespace Codebelt.Extensions.Xunit
{
internal class DelimitedStringOptions<T>
{
internal DelimitedStringOptions()
{
Delimiter = ",";
StringConverter = o => o.ToString();
}

internal Func<T, string> StringConverter { get; set; }

internal string Delimiter { get; set; }
}
}
5 changes: 3 additions & 2 deletions src/Codebelt.Extensions.Xunit/InMemoryTestStore.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Cuemon;

namespace Codebelt.Extensions.Xunit
{
Expand Down Expand Up @@ -49,7 +48,9 @@ public void Add(T item)
/// <returns>An <see cref="IEnumerable{T}" /> that satisfies the condition.</returns>
public virtual IEnumerable<T> Query(Func<T, bool> predicate = null)
{
return Condition.TernaryIf(predicate == null, () => _store, () => _store.Where(predicate!));
return predicate == null
? _store
: _store.Where(predicate!);
}

/// <summary>
Expand Down
5 changes: 2 additions & 3 deletions src/Codebelt.Extensions.Xunit/TestOutputHelperExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Cuemon;
using Xunit.Abstractions;

namespace Codebelt.Extensions.Xunit
Expand All @@ -21,7 +20,7 @@ public static class TestOutputHelperExtensions
/// </exception>
public static void WriteLines(this ITestOutputHelper helper, params object[] values)
{
Validator.ThrowIfNull(helper);
if (helper == null) { throw new ArgumentNullException(nameof(helper)); }
helper.WriteLine(DelimitedString.Create(values, o => o.Delimiter = Environment.NewLine));
}

Expand All @@ -48,7 +47,7 @@ public static void WriteLines<T>(this ITestOutputHelper helper, T[] values)
/// </exception>
public static void WriteLines<T>(this ITestOutputHelper helper, IEnumerable<T> values)
{
Validator.ThrowIfNull(helper);
if (helper == null) { throw new ArgumentNullException(nameof(helper)); }
helper.WriteLine(DelimitedString.Create(values, o => o.Delimiter = Environment.NewLine));
}
}
Expand Down