Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 1 minor |
🟢 Metrics 38 complexity
Metric Results Complexity 38
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: Fix item-at Satisfies source typing (#5706)
The fix is correct and clearly addresses the root cause: Satisfies was constructing a ValueAssertion<TItem> (a bare wrapper over IAssertionSource<TItem>) rather than a richly-typed assertion source, so users of collection-type items lost all the specialized assertion API.
Architecture concern: combinatorial overload explosion
ListItemAtSatisfiesExtensions.cs is 311 lines covering 11 concrete collection types × 2 source types = 22 overloads. This approach works, but it won't scale. Every time a new assertion type is added to TUnit (e.g. SortedSetAssertion, ImmutableArrayAssertion, or any user-defined collection assertion type), this file must be updated with 2 more overloads. The pattern also doesn't compose — it says nothing about IList<IList<IList<T>>> or other nested structures.
A more maintainable design would expose a single generic overload that accepts a source factory:
// Single entry point — no enumeration of concrete types needed
public static ListItemAtSatisfiesAssertion<TList, TItem> Satisfies<TList, TItem, TSource>(
this ListItemAtSource<TList, TItem> source,
Func<TItem, string, TSource> sourceFactory,
Func<TSource, IAssertion?> assertion,
[CallerArgumentExpression(nameof(assertion))] string? expression = null)
where TList : IList<TItem>
where TSource : IAssertionSource<TItem>
{
// one implementation
}Usage becomes:
await Assert.That(items).ItemAt(0)
.Satisfies(
(item, label) => new CollectionAssertion<int>(item, label),
a => a.Count().IsEqualTo(3));This pushes the factory responsibility to the call site, but TUnit could still ship convenience helpers (e.g. static factory methods on each assertion type) rather than enumerating every combination. The key benefit: users can bring their own assertion source types and they'll work automatically — the current approach gives them no way to do that.
internal accessors as a design smell
// ListAssertions.cs (and ReadOnlyListAssertions.cs)
internal AssertionContext<TList> InternalListContext => _listContext;
internal int InternalIndex => _index;These were private before; they're now exposed internal solely to allow the extension class (in the same assembly) to bypass normal construction. The Internal prefix signals this awkwardness. A cleaner alternative: a factory/builder method directly on ListItemAtSource<TList, TItem> that creates the assertion given a raw factory:
internal ListItemAtSatisfiesAssertion<TList, TItem> CreateSatisfiesAssertion(
Func<TItem, int, IAssertion?> factory)
{
_listContext.ExpressionBuilder.Append(...)
return new ListItemAtSatisfiesAssertion<TList, TItem>(_listContext, _index, factory);
}This hides the internals and keeps the responsibility for mutating ExpressionBuilder inside the source type, where it belongs.
The internal assertion constructor is unguarded on the expression side
The new internal constructor of ListItemAtSatisfiesAssertion skips the expression-appending that SatisfiesSpecialised does manually:
// Extension helper appends expression here...
source.InternalListContext.ExpressionBuilder.Append($".Satisfies({expression})");
// ...then constructs via internal ctor which does NOT append anything
return new ListItemAtSatisfiesAssertion<TList, TItem>(
source.InternalListContext,
source.InternalIndex,
assertionFactory);If anyone constructs ListItemAtSatisfiesAssertion via the internal ctor without calling ExpressionBuilder.Append first, the expression will be silently incomplete. The factory-method approach above would eliminate this risk by co-locating both steps.
Test coverage gaps
The regression tests cover String, Collection, Dictionary, and Set for both IList and IReadOnlyList — solid baseline. Missing:
T[](theArrayAssertionoverload is present but untested)HashSet<T>(theHashSetAssertionoverload exists)Dictionary<K,V>concrete type (there's aMutableDictionaryAssertionoverload for it)
Given this is a regression fix, having at least a smoke test for each overload variant that was added would make future regressions much easier to catch.
Summary
The core fix is sound — changing the internal delegate from Func<IAssertionSource<TItem>, Assertion<TItem>?> to Func<TItem, int, IAssertion?> and building the specialized source at dispatch time is exactly right. The backward-compatible forwarding constructor preserves the existing public API cleanly.
The concern is that the mechanism chosen (enumerating all known concrete types as overloads) trades one problem (missing specialization) for another (a maintenance tax on every new assertion type). Worth considering whether a single generic sourceFactory parameter would be a better long-term design before this pattern spreads to other assertion combinators.
|
Addressed the review feedback in ce722f9:
Verification rerun:
|
There was a problem hiding this comment.
Code Review: Fix item-at Satisfies source typing (#5706)
The PR correctly identifies and fixes the root cause: ListItemAtSatisfiesAssertion was storing Func<IAssertionSource<TItem>, Assertion<TItem>?> which forced the lambda return type to be Assertion<TItem>? — making it impossible to use collection assertions that chain to a different Assertion<T> (e.g., Count().IsEqualTo(3) returns Assertion<int>, not Assertion<IEnumerable<int>>).
What's done well
- The internal refactor to
Func<TItem, int, IAssertion?>is clean and separates item retrieval from assertion creation correctly. - Constructor chaining preserves backward compatibility elegantly.
- The
CreateSatisfiesAssertionbridge approach (following feedback from the previous review) correctly avoids exposing internal index/context accessors. - Good test coverage across diverse concrete types.
- Public API snapshots updated properly.
Architectural Concern: The Root Fix May Be Simpler
The 303-line extension file is working around a return-type covariance constraint on the existing overload. The original signature requires Func<IAssertionSource<TItem>, Assertion<TItem>?>, but collection assertion chains return Assertion<int> (not Assertion<IEnumerable<int>>), so the lambda won't compile.
If most collection assertion extension methods (like Count()) are already defined on IAssertionSource<IEnumerable<T>> — rather than only on the concrete CollectionAssertion<T> — then adding a single overload with a relaxed return type directly on the Satisfies method would handle the majority of cases without any per-type dispatch:
// One new overload on ListItemAtSource<TList, TItem>
public ListItemAtSatisfiesAssertion<TList, TItem> Satisfies(
Func<IAssertionSource<TItem>, IAssertion?> assertion,
[CallerArgumentExpression(nameof(assertion))] string? expression = null)
{
return CreateSatisfiesAssertion(
(item, index) => assertion(new ValueAssertion<TItem>(item, $"item[{index}]")),
expression);
}With this, users can write item => item.Count().IsEqualTo(3) and the lambda compiles because IAssertion? is satisfied by the Assertion<int> return. The full 303-line extension file would only be needed if users specifically need the concrete assertion type (e.g., CollectionAssertion<int> has methods not available through IAssertionSource<IEnumerable<int>>).
Is the extension file truly necessary, or is the IAssertion? overload sufficient for all the test cases?
Scalability of the Extension File
If the per-type extension methods are genuinely needed, the static dispatch approach has inherent growth pressure:
- Missing today:
ICollection<T>,Queue<T>,Stack<T>,LinkedList<T>,SortedList<TKey,TValue>,ImmutableArray<T>,ImmutableList<T>, etc. - Any user-defined assertion source type will never work with this approach.
A more future-proof alternative for strongly-typed sources would be a factory overload:
public ListItemAtSatisfiesAssertion<TList, TItem> Satisfies<TSource>(
Func<TItem, string, TSource> sourceFactory,
Func<TSource, IAssertion?> assertion,
[CallerArgumentExpression(nameof(assertion))] string? expression = null)
where TSource : IAssertionSource<TItem>Usage:
.Satisfies(
(item, name) => new CollectionAssertion<int>(item, name),
col => col.Count().IsEqualTo(3))This covers all current and future assertion source types without adding overloads, at the cost of a slightly more verbose call site. The per-type extension methods could still be provided as ergonomic wrappers for the most common types.
Minor
- The
stringitem path works correctly through the existingFunc<IAssertionSource<TItem>, Assertion<TItem>?>overload (string assertions returnAssertion<string>). The behavior asymmetry between string items and collection items is worth a code comment noting why no specialized overload is needed forstring. #if NET5_0_OR_GREATERonIReadOnlySet<T>is correct; no issues there.
Overall the implementation is functionally correct and the previous review feedback has been addressed. The main open question is whether the simpler single-overload approach (Func<IAssertionSource<TItem>, IAssertion?>) would suffice, which would let you drop most of the extension file. If the specialized concrete types are genuinely needed for their extra methods, the factory overload pattern would scale better than the current per-type enumeration.
Replace per-type Satisfies overload bodies with delegators through a new static-abstract factory interface. Interface-shape overloads collapse to one-liners; concrete-type overloads (List<T>, Dictionary<K,V>) keep direct construction since their assertion sources implement IAssertionSourceFor against the matching interface, not the concrete type. Pattern is reusable for the remaining #5703-5707 family (Member, Count, etc.) — each new target needs only the generic Satisfies entry plus delegators, no per-shape construction logic. netstandard2.0 drops the specialised overloads (no static abstracts); net8+/net9+/net10+ get the full surface.
There was a problem hiding this comment.
Code Review
Previous feedback (constructor chaining, CreateSatisfiesAssertion encapsulation, test coverage gaps) has been addressed. The architecture is sound.
What's working well
IAssertionSourceFor<TItem, TSelf> CRTP interface — The right foundation. Using C# 11 static abstract members for a per-item factory enables fully generic dispatch without per-type overloads at the core level. The constraint where TSource : IAssertionSourceFor<TItem, TSource> correctly closes the loop.
CreateSatisfiesAssertion encapsulation — Resolves the previous concern about leaking InternalListContext/InternalIndex. The extension methods call an internal method from within the same assembly, which is the correct scope.
Backward-compatible constructor chaining — The old public ListItemAtSatisfiesAssertion(context, index, Func<IAssertionSource<TItem>, Assertion<TItem>?>) constructor chains to the new internal one. No binary breaking change, and the wrapping via new ValueAssertion<TItem> is transparent to callers.
Func<TItem, int, IAssertion?> — Passing index alongside item is essential for constructing the label $"item[{index}]" in the right place, rather than deferring label construction to CheckAsync. This is the key fix for #5706.
Remaining design concern: List<T> / Dictionary<TKey,TValue> bridging asymmetry
The comment explains the why well, but there is a cleanable root cause. ListAssertion<T> implements IAssertionSourceFor<IList<T>, ListAssertion<T>> (interface-typed), not IAssertionSourceFor<List<T>, ListAssertion<T>> (concrete-typed). So the generic Satisfies<TSource> path can't bind when the item is List<T>, forcing the explicit bridge:
// Current — two code paths
=> source.Satisfies<ListAssertion<TInner>>(assertion, expression); // via IAssertionSourceFor
=> source.CreateSatisfiesAssertion( // manual bridge
(item, index) => assertion(new ListAssertion<TInner>(item, ...)), expression);ListAssertion<T> could implement both interface variants without conflict, since the Create static methods differ in parameter type (IList<T> vs List<T>):
public class ListAssertion<TItem>
: ListAssertionBase<IList<TItem>, TItem>
#if !NETSTANDARD2_0
, IAssertionSourceFor<IList<TItem>, ListAssertion<TItem>> // existing
, IAssertionSourceFor<List<TItem>, ListAssertion<TItem>> // new — eliminates bridge
#endif
{
public static ListAssertion<TItem> Create(IList<TItem> item, string label) => new(item, label);
public static ListAssertion<TItem> Create(List<TItem> item, string label) => new(item, label);
}C# can implement two parameterizations of the same generic interface when the static abstract member signatures are distinct (different TItem → different parameter type). With this, the concrete List<T> extension overloads would reduce to simple Satisfies<ListAssertion<TInner>> calls, eliminating the asymmetric CreateSatisfiesAssertion bridge and making the code more consistent. Same pattern applies to MutableDictionaryAssertion. Not a blocker, but worth a follow-up — the inconsistency will likely surface again when users try to add their own concrete-type overloads.
Minor: extension duplication between list / readonly-list source types
Each of the 11 logical type mappings appears twice in ListItemAtSatisfiesExtensions.cs — once for ListItemAtSource<TList, TItem> and once for ReadOnlyListItemAtSource<TList, TItem>. This is intrinsic because the return types differ (ListItemAtSatisfiesAssertion vs ReadOnlyListItemAtSatisfiesAssertion). A shared interface over both source types exposing CreateSatisfiesAssertion could halve this, but the return-type variance makes that non-trivial. Noting for visibility rather than blocking the PR.
Test coverage note
Tests cover IEnumerable<int>, int[], IDictionary<string,int>, Dictionary<string,int>, ISet<int>, HashSet<int>, IReadOnlyDictionary<string,int>, IReadOnlySet<int>, and string. The IList<T> and IReadOnlyList<T> item overloads (which are among the cleanest — pure Satisfies<TSource> delegation) don't appear to have dedicated test cases. Low risk, but worth adding for completeness.
Summary
Functionally correct, all previous feedback has been addressed, and the IAssertionSourceFor CRTP foundation is genuinely extensible — users can write Satisfies<MyCustomAssertion>(...) for types TUnit doesn't know about. The main open item is the List<T>/Dictionary<TKey,TValue> concrete-type bridging asymmetry, which is cosmetic now but becomes a maintenance tax as the pattern propagates.
ListAssertion and MutableDictionaryAssertion now implement two IAssertionSourceFor parameterisations each (interface + concrete item types), enabling the concrete List<T> and Dictionary<K,V> Satisfies overloads to delegate through the generic Satisfies<TSource> path instead of bypassing it via direct CreateSatisfiesAssertion calls. This eliminates the per-type construction asymmetry that would otherwise propagate to every future combinator following the same pattern. IAssertionSourceFor no longer extends IAssertionSource<TItem> — the framework only ever calls TSource.Create(...), and the multi-variant implementation is what required dropping the base (an IAssertionSource<List<T>> Context property cannot be derived from AssertionContext<IList<T>> without invariance breakage). Concrete classes still expose IAssertionSource<TItem> through their existing base hierarchies, so user lambdas see no surface change. Adds nested-list test coverage: IList<IList<T>>, IList<IReadOnlyList<T>>, IReadOnlyList<IList<T>>, IReadOnlyList<IReadOnlyList<T>>.
There was a problem hiding this comment.
Code Review: Fix item-at Satisfies source typing — iteration 4
All prior feedback has been fully addressed. This review covers only the new commit (0080687e).
What changed since the last review
The latest commit does three things:
IAssertionSourceFor<TItem, TSelf>no longer extendsIAssertionSource<TItem>— required to allowListAssertion<T>andMutableDictionaryAssertion<T>to implement two parameterizations of the interface simultaneously.ListAssertion<T>andMutableDictionaryAssertion<TKey,TValue>implement both the interface-typed and concrete-typed variants (IAssertionSourceFor<IList<T>>andIAssertionSourceFor<List<T>>), so the concrete overloads in the extension file now delegate through the genericSatisfies<TSource>path without anyCreateSatisfiesAssertionbypass.- Test coverage added for
IList<IList<T>>,IList<IReadOnlyList<T>>,IReadOnlyList<IList<T>>, andIReadOnlyList<IReadOnlyList<T>>— directly addresses the gap flagged in the last review.
All three of these were explicitly flagged as open items in the previous review, so well done on the quick turnaround.
Removing IAssertionSourceFor : IAssertionSource<TItem>
This is the right call. The commit message explains the invariance problem precisely, and the <remarks> doc on the interface explains the design rationale for future maintainers. The public API snapshot reflects the change correctly. Since IAssertionSourceFor was never shipped in a release, removing the inheritance is not a breaking change.
One small thing to verify: the API snapshots show that ArrayAssertion<TItem>, CollectionAssertion<TItem>, ReadOnlyListAssertion<TItem>, etc. now list fewer interfaces in their type hierarchy (the transitively-inherited IAssertionSource and IAssertionSource<TItem> that came from IAssertionSourceFor are no longer shown). The underlying implementations are unaffected — those interfaces are still satisfied through the base class hierarchy — so this is a snapshot artifact, not a behavior regression.
Minor gap: IList<List<T>> case not tested
The refactored List<T> overloads now route through Satisfies<ListAssertion<TInner>> (via the new IAssertionSourceFor<List<T>, ListAssertion<T>> implementation). This is the code path most changed by this commit, yet it has no dedicated test. All existing tests use IList<IList<T>>, not IList<List<T>>. A minimal test case:
[Test]
public async Task List_ItemAt_Satisfies_Preserves_ConcreteList_Item_Source()
{
IList<List<int>> items = new List<List<int>> { new() { 1, 2, 3 } };
await Assert.That(items).ItemAt(0).Satisfies(item => item.Count().IsEqualTo(3));
}Same applies to IList<Dictionary<string,int>> (the concrete dictionary path). Low risk since the types are equivalent at the IList<T> / IDictionary<K,V> level, but a regression test for exactly the code path changed this round would be good practice.
Summary
The implementation is clean, the internal Satisfies<TSource> path is now used consistently for both concrete and interface item types, the CreateSatisfiesAssertion bypass is gone, and the test coverage is substantially improved. The only remaining suggestion is smoke tests for the List<T> and Dictionary<K,V> concrete-item paths that are specifically enabled by this commit. No blockers.
Adds the IList<List<int>> case the previous round missed — this is the concrete-list path now routed through Satisfies<ListAssertion<TInner>> via the new IAssertionSourceFor<List<T>, ListAssertion<T>> implementation. Also documents the why behind the interface- and concrete-shaped overload coexistence on ListItemAtSatisfiesExtensions: C# overload resolution is exact on TItem, so IList<List<int>> binds the List<TInner> overload and never the IList<TInner> one.
Code Review: Fix item-at Satisfies source typing (#5764)This is a well-structured fix for a real usability problem. The core issue is that What the PR does wellBackward compatibility is preserved correctly. The original public constructor
Expression string is correctly passed through. The
Issues and observations1.
|
Code ReviewThis PR is well-executed overall. The design is clean, the tests are comprehensive, and the commit history shows thoughtful incremental refinement. A few observations: ✅ What's goodStatic abstract CRTP pattern is the right choice here.
Test coverage is thorough: all eight collection shapes × both mutable and read-only variants × both
|
Three review-driven clarifications: 1. The IReadOnlySet<TInner> overload in CollectionItemSatisfiesExtensions now sits inside #if NET5_0_OR_GREATER, matching the guard on ReadOnlySetAssertion itself. Functionally equivalent on the current netstandard2.0;net8;net9;net10 multi-target, but prevents a future netstandard2.1 target from compiling against a type that wouldn't be declared. 2. IAssertionSourceFor.Create gets a remarks block stating that implementations may receive a null item when callers select from a collection that allows nulls — the contract is "construct a source", not "validate non-null". 3. The four backward-compat public constructors on *ItemAtSatisfiesAssertion / *LastItemSatisfiesAssertion classes get remarks explaining why they wrap into ValueAssertion<TItem> while the internal paths now go through the specialised IAssertionSourceFor factory — preserves binary compat for direct consumers from before the refactor.
There was a problem hiding this comment.
Code Review — PR #5764: Fix item-at Satisfies source typing
Overall this is a well-reasoned fix for issue #5706. The use of static abstract interface members (CRTP via IAssertionSourceFor<TItem, TSelf>) is the right tool here: no reflection, AOT-safe, and statically dispatched. The three-layer design (interface contracts → #if !NETSTANDARD2_0-guarded implementations → extension overloads) is sound and the netstandard2.0 fallback is handled correctly. Public API snapshots and backward-compatible constructors are included.
Below are the findings, roughly ordered by severity.
Design / Architecture
Significant test duplication (Issue5706Tests vs Issue5778Tests)
The two test classes are structurally identical (diff shows only ItemAt ↔ LastItem and class name changes across all 188 lines). 376 lines of near-copy tests add maintenance burden and make it easy for changes to fix one path without breaking the other. A parameterised helper or [Arguments] on a shared method would be more maintainable:
// Example approach using TUnit's data-driven tests
[Test]
[Arguments("ItemAt")]
[Arguments("LastItem")]
public async Task List_Satisfies_Preserves_Collection_Item_Source(string selectorName)
{
IList<IList<int>> items = new List<IList<int>> { new List<int> { 1, 2, 3 } };
var selector = selectorName == "ItemAt"
? Assert.That(items).ItemAt(0)
: Assert.That(items).LastItem();
await selector.Satisfies(item => item.Count().IsEqualTo(3));
}No failure-path tests
All added tests only assert the happy path (assertion passes). There are no tests verifying that when the inner assertion fails, the error message is meaningful. For example: what does the user see when item.Count().IsEqualTo(3) is applied to new[] {1, 2} inside a Satisfies call? Given the "Assert.That(item[0])." prefix that all source constructors emit, the nested failure message could look odd. A single failure-path test per scenario would give confidence the error messages are user-friendly.
Correctness
"Assert.That(item[N])" prefix on inner assertion sources (pre-existing, but amplified)
CollectionAssertion.Create(item, "item[0]") calls new CollectionAssertion(item, "item[0]"), which calls CreateExpressionBuilder("item[0]"), which appends "Assert.That(item[0])". This means a failed inner assertion like item.Count().IsEqualTo(3) produces an error containing "Assert.That(item[0]).Count() ..." nested inside the outer "Assert.That(items).ItemAt(0).Satisfies(...)" context. While this is pre-existing behaviour (the old new ValueAssertion<TItem>(item, ...) path had the same prefix), it is more visible now that specialised assertion types are used. It is not a blocker but worth noting for a future pass.
ISet<T>.IsSupersetOf/IReadOnlySet<T>.IsSupersetOf resolve correctly — but only incidentally
The IItemSatisfiesSource docs explain it is needed to prevent Satisfies(Func<TValue?, bool>) winning when item assertion methods return bool. The Set tests call item.IsSupersetOf(...) where item is SetAssertion<int> whose IsSupersetOf returns SetIsSupersetOfAssertion (IAssertion), so they go through the correct specialised path. This is correct, but it is easy to add a future test where a set assertion method does return bool, silently dropping back to the predicate path. A comment directly on the set overloads in CollectionItemSatisfiesExtensions would help.
Minor / Nits
CreateSatisfiesAssertion visibility
internal ListItemAtSatisfiesAssertion<TList, TItem> CreateSatisfiesAssertion(...) is internal rather than private. This is fine for within-assembly use but, since it is only called by the two Satisfies overloads in the same class, making it private would tighten the API surface. If testability was the reason for internal, a comment would clarify intent.
Missing [Obsolete] on the backward-compatible public constructors
/// <summary>
/// Backward-compatible constructor preserved for direct consumers from before the
/// internal refactor to a raw-item delegate. ...
/// </summary>
public ListItemAtSatisfiesAssertion(
AssertionContext<TList> context,
int index,
Func<IAssertionSource<TItem>, Assertion<TItem>?> assertion)The doc comment says "Internal callers should use Satisfies<TSource>" and this is included in the verified public API, so it is accessible to third-party consumers. If the intent is that external callers should not reach for this constructor directly in new code, an [Obsolete] with a message pointing to the Satisfies<TSource> entry-point would communicate that clearly. If it is legitimately the intended external API, the doc comment should be updated to say so.
IItemSatisfiesSource.Satisfies lacks [CallerArgumentExpression]
TResult Satisfies<TSource>(Func<TSource, IAssertion?> assertion, string? expression = null)The interface method cannot carry [CallerArgumentExpression] in a way that C# honours on interface implementations. The extension methods in CollectionItemSatisfiesExtensions correctly capture the expression and pass it explicitly, so this works end-to-end. Worth a brief comment on the interface explaining why [CallerArgumentExpression] is omitted, to prevent a future contributor adding it and expecting it to work.
CollectionItemSatisfiesExtensions — missing XML doc on individual overloads
The class-level <summary> is thorough, but none of the eleven public extension methods have individual <summary> tags. The public API snapshot will suppress CS1591, so this is not a build issue, but IDE hover-doc for callers would be blank. Even a single shared /// <inheritdoc cref="IItemSatisfiesSource{TItem,TResult}.Satisfies{TSource}"/> on each would help.
label vs expression parameter naming
IAssertionSourceFor<TItem, TSelf>.Create(TItem item, string label) uses the name label, while all existing source constructors use expression. This is internally consistent (items-within-collections are not user-typed expressions) but the difference may surprise contributors who look at the symmetry between new ValueAssertion<T>(item, expr) and TSource.Create(item, label). A brief note in the IAssertionSourceFor XML doc would prevent confusion.
Summary
The core fix is correct, the abstractions are clean, the AOT story is good, and the public API snapshots are updated. The main asks before merging would be: (1) at least one failure-path test to verify error message quality, and (2) collapsing the duplicate test classes. The rest are suggestions for ongoing improvement.
1 participant
Summary
Verification
Closes #5706