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

add simple comments #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
6 changes: 4 additions & 2 deletions NUnit.Middlewares.Tests/ParallelTestFixtureSetUpTest.cs
Original file line number Diff line number Diff line change
@@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -68,4 +70,4 @@ public void Test(int n)
((Counter)item3!).InvocationsCount.Should().Be(0);
}
}
}
}
4 changes: 2 additions & 2 deletions NUnit.Middlewares.Tests/TestWithRetriesHasItsOwnSetup.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;

using FluentAssertions;

Expand Down Expand Up @@ -90,4 +90,4 @@ protected override void Configure(ISetupBuilder fixture, ISetupBuilder test)
});
}
}
}
}
4 changes: 2 additions & 2 deletions NUnit.Middlewares/CompositeSetup.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
Expand Down Expand Up @@ -52,4 +52,4 @@ public async Task TearDownAsync(ITest test)

private readonly List<SetUpAsync<TearDownAsync>> setups;
}
}
}
4 changes: 2 additions & 2 deletions NUnit.Middlewares/ISetupBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
namespace SkbKontur.NUnit.Middlewares
namespace SkbKontur.NUnit.Middlewares
{
public interface ISetupBuilder
{
ISetupBuilder Use(SetUpAsync<TearDownAsync> setup);
ISetup Build();
}
}
}
5 changes: 3 additions & 2 deletions NUnit.Middlewares/SetupDelegates.cs
Original file line number Diff line number Diff line change
@@ -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<out TTearDown>(ITest test);
Expand All @@ -15,4 +16,4 @@ namespace SkbKontur.NUnit.Middlewares
public delegate void TearDown();

public delegate Task TearDownAsync();
}
}
5 changes: 3 additions & 2 deletions NUnit.Middlewares/SimpleTestContext.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Generic;
using System.Collections.Generic;

using NUnit.Framework.Interfaces;
using NUnit.Framework.Internal;
Expand All @@ -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<object>? this[string key] => (IReadOnlyList<object>?)test.ListRecursive(key);

private readonly ITest test;
}
}
}
8 changes: 6 additions & 2 deletions NUnit.Middlewares/SimpleTestContextExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections;
using System.Collections.Generic;

Expand Down Expand Up @@ -34,6 +34,8 @@ public static T Get<T>(this SimpleTestContext context)
return context.Get<T>(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<T>(this SimpleTestContext context, string key)
{
return (T?)context.Get(key)
Expand All @@ -43,6 +45,7 @@ public static T Get<T>(this SimpleTestContext context, string key)
public static void Set<T>(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);
}

Expand All @@ -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) =>
Expand All @@ -78,4 +82,4 @@ public static bool ContainsKeyRecursive(this ITest test, string key) =>
return default;
}
}
}
}