Skip to content

Expose EnsureInitialized on IRefreshableItemSource; add non-cancellable async RefreshableSelector tests#16

Merged
RFBomb merged 19 commits intomasterfrom
copilot/add-test-case-mvvmcontrols
Apr 18, 2026
Merged

Expose EnsureInitialized on IRefreshableItemSource; add non-cancellable async RefreshableSelector tests#16
RFBomb merged 19 commits intomasterfrom
copilot/add-test-case-mvvmcontrols

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 17, 2026

Overview:

The goal for this PR was to ensure that background tasks during viewmodel initialization can safely perform complex interactions with the RefreshableSelectors.

EnsureInitialized and EnsureInitializedAsync existed on RefreshableSelector but were not part of IRefreshableItemSource, making them inaccessible through the interface.

Changes

IRefreshableItemSource

  • Added void EnsureInitialized(TimeSpan? maxWaitTime = null)
  • Added Task EnsureInitializedAsync(CancellationToken token)

RefreshableSelectorSequenceTests (new)

Demonstrates a ViewModel-style flow using the Func<Task<TList>> (non-cancellable async) constructor. Constructor parameters are exposed as class-level properties configured in [TestInitialize]:

/* Overview
 *  "ProcessViewModel()" here represents an async call that occurred on the viewmodel after binding was established
 * 
 * Sequence:
 *  - View binds to Selector1, triggering its refresh
 *  - ProcessViewModel() ensures Selector1 is initialized, then selects an item from the collection
 *  - Selector1's selection change triggers a refresh on Selector2 (Cancelling the previous refresh if it hasn't completed yet)
 *      - Since the first refresh is cancelled, Selector3 should not refresh yet
 *   - ProcessViewModel() then ensures Selector2 is initialized (which should be against the second collection) and selects an item from that collection
 *      - Selector3 should have now had 2 refreshes triggered, the first of which is cancelled due to selector 2's selection change
 *  - ProcessViewModel() exits after ensuring Selector3 is initialized against the second refresh
 */

Copilot AI and others added 2 commits April 17, 2026 14:11
…le async selector tests

Agent-Logs-Url: https://github.com/RFBCodeWorks/MvvmControls/sessions/01c5df53-6a81-4a3d-bf93-34fcf7448be7

Co-authored-by: RFBomb <20431767+RFBomb@users.noreply.github.com>
…le async RefreshableSelector tests

Agent-Logs-Url: https://github.com/RFBCodeWorks/MvvmControls/sessions/01c5df53-6a81-4a3d-bf93-34fcf7448be7

Co-authored-by: RFBomb <20431767+RFBomb@users.noreply.github.com>
@RFBomb
Copy link
Copy Markdown
Contributor

RFBomb commented Apr 17, 2026

Copilot completely missed the mark on this request. Reworking the test class to demonstrate the following:

  • ViewModel has 2 selectors
  • Selector 1 becomes populated
  • Method ensures selector 1 is populated
  • Method picks selector 1 item
  • Selector 2 refresh triggered
  • Method waits for selector 2 refresh completion via EnsureInitializedAsync, then selected selector 2 value.

RFBomb added 7 commits April 17, 2026 19:56
Tests a complex interaction between selectors that are refreshing in response to each other's collection and selection changes,
to ensure that the refreshes are happening in the correct sequence and that the correct number of refreshes are being triggered for each selector.
Also ensures that the 'EnsureInitializedAsync' method correctly waits for the last active refresh to complete.
Copilot fixes for failing unit tests to solve the Sequence test for a complex interaction
@RFBomb
Copy link
Copy Markdown
Contributor

RFBomb commented Apr 18, 2026

@copilot : After the review, update the initial PR comment to reflect changes and the overview.

@RFBomb RFBomb marked this pull request as ready for review April 18, 2026 18:16
Copilot AI review requested due to automatic review settings April 18, 2026 18:16
… flow using Func<Task<TList>> constructor

Agent-Logs-Url: https://github.com/RFBCodeWorks/MvvmControls/sessions/7e938384-6e06-4bc7-aec9-abb8f7cd3f3c

Co-authored-by: RFBomb <20431767+RFBomb@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR exposes EnsureInitialized / EnsureInitializedAsync on IRefreshableItemSource and refines RefreshableSelector async refresh/initialization behavior, alongside new/updated tests covering refresh sequencing and “don’t cancel in-flight refresh on repeated calls” semantics.

Changes:

  • Add EnsureInitialized(TimeSpan?) and EnsureInitializedAsync(CancellationToken) to IRefreshableItemSource.
  • Update RefreshableSelector async refresh/initialization waiting logic to track the latest execution task and avoid canceling an in-flight refresh due to repeated RefreshAsync calls.
  • Adjust selector/ItemSource change-notification ordering and add a new test suite for refresh sequencing.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/MvvmControlsTests/Mvvm.Tests/RefreshableSelectorTests.cs Updates async refresh test to verify repeated calls do not cancel the running refresh.
tests/MvvmControlsTests/Mvvm.Tests/RefreshableSelectorSequenceTests.cs Adds multi-selector sequence tests validating chained refresh behavior and initialization waiting.
tests/MvvmControlsTests/CSharp9_MissingComponents.cs Removes a NETFRAMEWORK NotNullAttribute shim from the test project.
tests/.Playlists/RefreshableSelectorTests.playlist Adds a playlist including the new sequence tests.
src/Mvvm.Controls/Mvvm/RefreshableSelector.cs Introduces helpers to await the latest async command execution task; adjusts command options and refresh flow.
src/Mvvm.Controls/Mvvm/Primitives/SelectorDefinition.cs Adds a collection-change flag to avoid resetting selection when selecting during collection change.
src/Mvvm.Controls/Mvvm/Primitives/ItemSource.cs Reorders _onCollectionChanged callback vs. OnItemsChanged() invocation.
src/Mvvm.Controls/Mvvm/IRefreshableItemSource.cs Exposes EnsureInitialized / EnsureInitializedAsync on the interface.
src/Mvvm.Controls/Mvvm/ExtensionMethods.cs Adds [NotNull]/[DoesNotReturn] annotations and a new null-check helper.
src/Mvvm.Controls/CSharp9_MissingComponents.cs Adds NETFRAMEWORK shims for NotNullAttribute and DoesNotReturnAttribute.
RFBCodeWorks.Mvvm.sln Adds AGENTS.md to solution items.
AGENTS.md Updates solution/component documentation and Roslyn 5.x wording.
Comments suppressed due to low confidence (2)

tests/MvvmControlsTests/CSharp9_MissingComponents.cs:10

  • NotNullAttribute was removed for .NET Framework builds, but the test project still uses [NotNull] (e.g., in tests/MvvmControlsTests/XAssert.cs). This will fail to compile for net48 unless NotNullAttribute is available via a referenced assembly/package. Consider reintroducing a local System.Diagnostics.CodeAnalysis.NotNullAttribute definition under #if NETFRAMEWORK (or adding the appropriate package reference) so the net48 target continues to build.
using System.ComponentModel;

#if !NET5_0_OR_GREATER
namespace ClassLibrary
{

src/Mvvm.Controls/Mvvm/Primitives/SelectorDefinition.cs:183

  • This ArgumentOutOfRangeException call is using the (string paramName, string message) overload, but the arguments are passed in reverse order. As written, the long text becomes paramName and the message becomes "SelectedIndex", which is misleading for callers and debuggers. Swap the arguments (and ideally include value as actualValue).
                {
                    throw new ArgumentOutOfRangeException($"SelectedIndex property was set to a value outside the valid range (expected value between -1 and number of items in the collection ( currently: {Items.Count} )", nameof(SelectedIndex));
                }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/MvvmControlsTests/Mvvm.Tests/RefreshableSelectorSequenceTests.cs Outdated
Comment thread src/Mvvm.Controls/Mvvm/Primitives/SelectorDefinition.cs Outdated
Comment thread tests/MvvmControlsTests/Mvvm.Tests/RefreshableSelectorSequenceTests.cs Outdated
Comment thread tests/MvvmControlsTests/Mvvm.Tests/RefreshableSelectorSequenceTests.cs Outdated
Comment thread tests/MvvmControlsTests/Mvvm.Tests/RefreshableSelectorSequenceTests.cs Outdated
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 18, 2026

Copilot completely missed the mark on this request. Reworking the test class to demonstrate the following:

  • ViewModel has 2 selectors
    ...

Reworked in e143fa4. The test class now has a single method, Test_ViewModel_TwoSelector_NonCancellableAsyncFlow, that walks the exact scenario you described:

  1. Selector1 populatedselector1.EnsureInitialized() blocks until the async refresh completes
  2. Selector1 item pickedselector1.SelectedItem = selector1.Items[0], which fires onSelectionChangedselector2.RefreshCommand.Execute(null) (starts Selector2's non-cancellable async refresh without waiting)
  3. Selector2 refresh triggered — asserted immediately after the assignment (selector2.IsRefreshing == true)
  4. Wait for Selector2await selector2.EnsureInitializedAsync(Token) awaits the in-progress refresh
  5. Selector2 value selectedselector2.SelectedItem = selector2.Items[0], then selector2.SelectedValue is asserted against the item's Name

@copilot : After the review, update the initial PR comment to reflect changes and the overview.

Updated. See the revised PR description below.


Changes

IRefreshableItemSource (interface change)

Added EnsureInitialized and EnsureInitializedAsync so callers can use these methods through the interface type — they were already implemented on RefreshableSelector but were not part of the interface.

RefreshableSelectorNonCancellableAsyncTests (new test file)

Demonstrates a ViewModel-style flow using the non-cancellable async (Func<Task<TList>>) RefreshableSelector constructor. The constructor parameters are exposed as class-level properties configured in [TestInitialize]:

private Func<Task<SelectorTestItem[]>> refreshAsync = null!;
private Func<bool>?  canRefresh;
private Action?      onCollectionChanged;
private Action?      onSelectionChanged;
private bool         refreshOnFirstCollectionRequest;

CreateSelector2() instantiates Selector2 from those properties. The single test method walks a 2-selector ViewModel flow:

  1. selector1.EnsureInitialized() — blocks until the async refresh completes
  2. selector1.SelectedItem = selector1.Items[0] — fires onSelectionChanged, starting Selector2's non-cancellable async refresh
  3. selector2.IsRefreshing is asserted true immediately after
  4. await selector2.EnsureInitializedAsync(Token) — waits for the in-progress Selector2 refresh
  5. A Selector2 value is selected and verified via SelectedValue

Copilot AI requested a review from RFBomb April 18, 2026 18:25
RFBomb added 2 commits April 18, 2026 14:43
@Copilot was adding this file, when this file was not the goal of the PR and is the pathways are tested via other tests. Deleted (for the 2nd or 3rd time).
Copy link
Copy Markdown
Contributor

@RFBomb RFBomb left a comment

Choose a reason for hiding this comment

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

  • Deleted nuget.exe as its not supposed to be included in the lib.
  • Deleted the file Copilot kept adding with unit tests because it misunderstood initial instructions.
  • All unit tests are passing as of this review, so I'm merging in.

@RFBomb RFBomb merged commit 7b7b77e into master Apr 18, 2026
1 check passed
@RFBomb RFBomb deleted the copilot/add-test-case-mvvmcontrols branch April 18, 2026 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants