diff --git a/NUnit.Middlewares.Tests/ParallelTestFixtureSetUpTest.cs b/NUnit.Middlewares.Tests/ParallelTestFixtureSetUpTest.cs index d941200..b6d61de 100644 --- a/NUnit.Middlewares.Tests/ParallelTestFixtureSetUpTest.cs +++ b/NUnit.Middlewares.Tests/ParallelTestFixtureSetUpTest.cs @@ -1,9 +1,10 @@ -using FluentAssertions; +using FluentAssertions; using NUnit.Framework; namespace SkbKontur.NUnit.Middlewares.Tests { + // sivukhin: (nit) some descriptions for test meaning will be very helpful (could be comment or maybe meaningful name) [Parallelizable(ParallelScope.Self)] public class FirstTestFixtureSetUpTest : SimpleTestBase { @@ -34,6 +35,7 @@ public void Test() } } + // sivukhin: this test looks strange as it always overwrite "item3" property with fresh new Counter() [Parallelizable(ParallelScope.Children)] public class ThirdTestFixtureSetUpTest : SimpleTestBase { @@ -68,4 +70,4 @@ public void Test(int n) ((Counter)item3!).InvocationsCount.Should().Be(0); } } -} \ No newline at end of file +} diff --git a/NUnit.Middlewares.Tests/TestWithRetriesHasItsOwnSetup.cs b/NUnit.Middlewares.Tests/TestWithRetriesHasItsOwnSetup.cs index 8500daf..6ced7a1 100644 --- a/NUnit.Middlewares.Tests/TestWithRetriesHasItsOwnSetup.cs +++ b/NUnit.Middlewares.Tests/TestWithRetriesHasItsOwnSetup.cs @@ -1,4 +1,4 @@ -using System; +using System; using FluentAssertions; @@ -90,4 +90,4 @@ protected override void Configure(ISetupBuilder fixture, ISetupBuilder test) }); } } -} \ No newline at end of file +} diff --git a/NUnit.Middlewares/CompositeSetup.cs b/NUnit.Middlewares/CompositeSetup.cs index 1c84334..e5d00ed 100644 --- a/NUnit.Middlewares/CompositeSetup.cs +++ b/NUnit.Middlewares/CompositeSetup.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; @@ -52,4 +52,4 @@ public async Task TearDownAsync(ITest test) private readonly List> setups; } -} \ No newline at end of file +} diff --git a/NUnit.Middlewares/ISetupBuilder.cs b/NUnit.Middlewares/ISetupBuilder.cs index b992f52..fa8289d 100644 --- a/NUnit.Middlewares/ISetupBuilder.cs +++ b/NUnit.Middlewares/ISetupBuilder.cs @@ -1,8 +1,8 @@ -namespace SkbKontur.NUnit.Middlewares +namespace SkbKontur.NUnit.Middlewares { public interface ISetupBuilder { ISetupBuilder Use(SetUpAsync setup); ISetup Build(); } -} \ No newline at end of file +} diff --git a/NUnit.Middlewares/SetupDelegates.cs b/NUnit.Middlewares/SetupDelegates.cs index 1bf827a..4ac9ef1 100644 --- a/NUnit.Middlewares/SetupDelegates.cs +++ b/NUnit.Middlewares/SetupDelegates.cs @@ -1,9 +1,10 @@ -using System.Threading.Tasks; +using System.Threading.Tasks; using NUnit.Framework.Interfaces; namespace SkbKontur.NUnit.Middlewares { + // sivukhin: should we add native support for cancellation in the core delegates signatures (pass CancellationToken in SetUpAsync / TearDownAsync?) public delegate void SetUp(ITest test); public delegate TTearDown SetUp(ITest test); @@ -15,4 +16,4 @@ namespace SkbKontur.NUnit.Middlewares public delegate void TearDown(); public delegate Task TearDownAsync(); -} \ No newline at end of file +} diff --git a/NUnit.Middlewares/SimpleTestContext.cs b/NUnit.Middlewares/SimpleTestContext.cs index 5c2ffff..4ee3e99 100644 --- a/NUnit.Middlewares/SimpleTestContext.cs +++ b/NUnit.Middlewares/SimpleTestContext.cs @@ -1,4 +1,4 @@ -using System.Collections.Generic; +using System.Collections.Generic; using NUnit.Framework.Interfaces; using NUnit.Framework.Internal; @@ -16,8 +16,9 @@ private SimpleTestContext(ITest test) public object? Get(string key) => test.GetRecursive(key); public bool ContainsKey(string key) => test.ContainsKeyRecursive(key); + // sivukhin: just curious, why define index operator here as a ListRecursive alias? Is it intuitive that indexing will return list? public IReadOnlyList? this[string key] => (IReadOnlyList?)test.ListRecursive(key); private readonly ITest test; } -} \ No newline at end of file +} diff --git a/NUnit.Middlewares/SimpleTestContextExtensions.cs b/NUnit.Middlewares/SimpleTestContextExtensions.cs index 358563a..8dde06f 100644 --- a/NUnit.Middlewares/SimpleTestContextExtensions.cs +++ b/NUnit.Middlewares/SimpleTestContextExtensions.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections; using System.Collections.Generic; @@ -34,6 +34,8 @@ public static T Get(this SimpleTestContext context) return context.Get(typeof(T).Name); } + // sivukhin: not all method which throws have OrThrow suffix + // Should we add it everywhere or maybe use another naming convention with pair of methods: Get / TryGet (or maybe even MustGet / TryGet)? public static T Get(this SimpleTestContext context, string key) { return (T?)context.Get(key) @@ -43,6 +45,7 @@ public static T Get(this SimpleTestContext context, string key) public static void Set(this IPropertyBag properties, T value) where T : notnull { + // sivukhin: should we add unique prefix for the key name derived from type name in order to avoid possible clashes in the PropertyBag? properties.Set(typeof(T).Name, value); } @@ -53,6 +56,7 @@ public static object GetRecursiveOrThrow(this ITest test, string key) } public static object? GetRecursive(this ITest test, string key) => + // sivukhin: (nit) static (p, k) => p.Get(k)? GetRecursive(test, key, (p, k) => p.Get(k)); public static bool ContainsKeyRecursive(this ITest test, string key) => @@ -78,4 +82,4 @@ public static bool ContainsKeyRecursive(this ITest test, string key) => return default; } } -} \ No newline at end of file +}