Enhances automated workflows and test reliability by michaelstonis · Pull Request #1 · michaelstonis/R3Ext

michaelstonis · 2025-11-29T04:53:22Z

No description provided.

* Grants automated workflows necessary permissions to manage repository contents, packages, and discussions. * Improves the reliability of asynchronous transformation cancellation tests. Replaces fixed delays with controlled task completion sources for more accurate verification of cancellation behavior.

…gnaling

…signaling

…sed signaling - Replace Task.Delay timing with TaskCompletionSource signaling - Use per-item TCS dictionaries for multi-value scenarios - Add StyleCop pragma for test file formatting - All 10 tests now use deterministic async coordination

…S-based signaling - Replace Task.Delay(10) settling waits with emission counting TCS pattern - All 7 tests now use deterministic async coordination - Add StyleCop pragma for test file formatting

- Replace timing-based test with synchronous emission order verification - Remove unnecessary async from synchronous tests - Add StyleCop pragma for test file formatting

- Add started signals for SelectLatestAsync cancellation test - Replace Task.Delay work simulation with TCS-based blocking - All 21 tests now fully deterministic

- Remove unnecessary Task.Delay from synchronous operations - Convert async tests to sync where no awaits needed - FakeTimeProvider tests are synchronous by design - Add StyleCop pragma for test file formatting

- TransformManyDedupTests: Replace polling with TCS signaling - RxCommandTests: Use TCS for async operation control - TransformAsyncCacheTests: Use per-item TCS for controlled completion - InteractionAdvancedTests: Replace Task.Delay with Task.Yield and TCS - AsyncIntegrationTests: Use TCS-based coordination - InteractionTests: Use Task.Yield for async execution All tests now use deterministic async coordination patterns

Copilot

Pull request overview

This PR enhances test reliability by replacing non-deterministic Task.Delay() calls with explicit synchronization using TaskCompletionSource, and adds explicit workflow permissions. The changes make tests more deterministic and less prone to timing-related failures in CI/CD environments.

Key changes:

Replaces Task.Delay() with TaskCompletionSource for controlled test synchronization
Removes unnecessary async modifiers from synchronous tests
Adds explicit permissions to GitHub Actions workflow

Reviewed changes

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

Show a summary per file

File	Description
R3Ext.Tests/TransformManyDedupTests.cs	Replaces polling helper with TaskCompletionSource-based synchronization for emission tracking
R3Ext.Tests/TransformAsyncCacheTests.cs	Introduces per-item TaskCompletionSource for controlled async transformation completion
R3Ext.Tests/SignalExtensionsAdvancedTests.cs	Removes async modifiers from synchronous tests and improves timing verification
R3Ext.Tests/RxCommandTests.cs	Replaces Task.Delay with TaskCompletionSource for async operation verification
R3Ext.Tests/RxCommandAdvancedTests.cs	Uses TaskCompletionSource to control execution flow in concurrent tests
R3Ext.Tests/InteractionTests.cs	Replaces Task.Delay(1) with Task.Yield() for minimal async behavior
R3Ext.Tests/InteractionAdvancedTests.cs	Adds controlled release mechanism for concurrent handler tests
R3Ext.Tests/ErrorHandlingAdvancedTests.cs	Removes unnecessary async modifiers and Task.Delay calls from synchronous tests
R3Ext.Tests/CacheOperatorParityPhase2Tests.cs	Implements emission counting with TaskCompletionSource for reliable test synchronization
R3Ext.Tests/AsyncIntegrationTests.cs	Uses blocking TaskCompletionSource instead of arbitrary delays for cancellation tests
R3Ext.Tests/AsyncExtensionsTests.cs	Adds per-item TaskCompletionSource dictionaries for fine-grained async operation control
R3.DynamicData.Tests/List/TransformAsyncTests.cs	Replaces Task.Delay with TaskCompletionSource and improves cancellation test reliability
.github/workflows/ci-cd.yml	Adds explicit permissions for contents, packages, and discussions

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

Copilot · 2025-11-29T04:57:46Z

R3Ext.Tests/TransformManyDedupTests.cs

-        Assert.Equal(2, changesReceived.Count); // No new change set emitted
+        // No new changeset expected - stay at count 2
+        Assert.Equal(2, changesReceived.Count);



Potential race condition: Same issue as lines 48-50. The expectedCount and emitTcs are updated before performing the action. Update these variables AFTER line 59's assertion (or add a small delay/synchronization point) to ensure the previous operation's callback has completed.

Suggested change

// Update synchronization variables after previous assertion, before triggering the next action

Copilot · 2025-11-29T04:57:46Z

R3Ext.Tests/CacheOperatorParityPhase2Tests.cs

+        expectedCount = 4;
+        emitTcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
+


Potential race condition: Same issue - expectedCount and emitTcs updated before the action on line 67. Move lines 64-65 after line 61's assertion.

Suggested change

expectedCount = 4;

emitTcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);

expectedCount = 4;

emitTcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);

Copilot · 2025-11-29T04:57:46Z

R3Ext.Tests/CacheOperatorParityPhase2Tests.cs

        cache.AddOrUpdate(new Item { Id = 1, Value = 1 });
-        await Task.Delay(10);
+        await emitTcs.Task.WaitAsync(TimeSpan.FromSeconds(5));



Potential race condition: The expectedCount and emitTcs are updated before the action on line 100. Move lines 98-99 after line 96's await.

Suggested change

// Move expectedCount and emitTcs update after previous await, before next emission

Copilot · 2025-11-29T04:57:47Z

R3Ext.Tests/CacheOperatorParityPhase2Tests.cs

-        await Task.Delay(10);
+        await emitTcs.Task.WaitAsync(TimeSpan.FromSeconds(5));
        Assert.True(results.Last().HasValue);



Potential race condition: The expectedCount and emitTcs are updated before the action on line 174. Move lines 172-173 after line 170's assertion.

Suggested change

Copilot · 2025-11-29T04:57:47Z

R3Ext.Tests/TransformManyDedupTests.cs

+        expectedCount = 3;
+        emitTcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
        cache.AddOrUpdate(new Item { Id = 1, Values = new List<int> { 2, 4 } }); // Update introducing 4


Potential race condition: Same pattern as earlier - expectedCount and emitTcs are updated before the action on line 97. Move these updates after line 93's assertion.

Copilot · 2025-11-29T04:57:49Z

R3Ext.Tests/TransformManyDedupTests.cs

+        expectedCount = 2;
+        emitTcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
        cache.AddOrUpdate(new Item { Id = 2, Values = new List<int> { 1, 2, 3 } });


Potential race condition: The expectedCount and emitTcs are updated before performing the action that triggers the emission. If the subscription callback from the previous operation (line 42) executes after these updates but before awaiting on line 43, it could see the new expectedCount value (2) and incorrectly signal emitTcs. This would cause the test to proceed before the second AddOrUpdate completes.

To fix this, update expectedCount and create the new emitTcs AFTER the await on line 43, right before the AddOrUpdate on line 50.

Copilot · 2025-11-29T04:57:49Z

R3.DynamicData.Tests/List/TransformAsyncTests.cs

+                foreach (var c in cs)
+                {
+                    if (c.Reason == ListChangeReason.Add)
+                    {
+                        if (Interlocked.Increment(ref addCount) == 3) addsDoneTcs.TrySetResult(true);
+                    }
+                }


This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot · 2025-11-29T04:57:49Z

R3.DynamicData.Tests/List/TransformAsyncTests.cs

            {
-                Interlocked.Increment(ref transformCount);
-                await Task.Delay(50);
+                var count = Interlocked.Increment(ref transformCount);


This assignment to count is useless, since its value is never read.

Suggested change

var count = Interlocked.Increment(ref transformCount);

Interlocked.Increment(ref transformCount);

Copilot · 2025-11-29T04:57:49Z

R3Ext.Tests/AsyncIntegrationTests.cs

    {
        Subject<int> subject = new();
        int completed = 0;
+        var completedTcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);


This assignment to completedTcs is useless, since its value is never read.

Suggested change

var completedTcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);

Copilot · 2025-11-29T04:57:50Z

R3.DynamicData.Tests/List/TransformAsyncTests.cs

+                    if (c.Reason == ListChangeReason.Add)
+                    {
+                        if (Interlocked.Increment(ref addCount) == 3) addsDoneTcs.TrySetResult(true);


These 'if' statements can be combined.

Suggested change

if (c.Reason == ListChangeReason.Add)

{

if (Interlocked.Increment(ref addCount) == 3) addsDoneTcs.TrySetResult(true);

if (c.Reason == ListChangeReason.Add && Interlocked.Increment(ref addCount) == 3)

{

addsDoneTcs.TrySetResult(true);

- RxCommandTests: Replace unreliable thread ID check with async behavior test - CacheOperatorParityPhase2Tests: Fix emission expectations for filtered items - AsyncExtensionsTests: Wait for each SelectLatestAsync result properly - TransformManyDedupTests: Apply safe TCS pattern with results.Count check - AsyncIntegrationTests: Remove unused completedTcs variable

michaelstonis and others added 10 commits November 27, 2025 13:28

Merge branch 'feature/cleanup' into develop

aefe5fb

fix(tests): make AsyncExtensionsTests deterministic with TCS-based si…

f9552c5

…gnaling

fix(tests): make RxCommandAdvancedTests deterministic with TCS-based …

3f74723

…signaling

fix(tests): make CacheOperatorParityPhase2Tests deterministic with TC…

3b406c5

…S-based signaling - Replace Task.Delay(10) settling waits with emission counting TCS pattern - All 7 tests now use deterministic async coordination - Add StyleCop pragma for test file formatting

fix(tests): make SignalExtensionsAdvancedTests deterministic

0097f9e

- Replace timing-based test with synchronous emission order verification - Remove unnecessary async from synchronous tests - Add StyleCop pragma for test file formatting

fix(tests): improve AsyncExtensionsTests with additional TCS fixes

bc7a2b9

- Add started signals for SelectLatestAsync cancellation test - Replace Task.Delay work simulation with TCS-based blocking - All 21 tests now fully deterministic

fix(tests): make ErrorHandlingAdvancedTests deterministic

c58f2c4

- Remove unnecessary Task.Delay from synchronous operations - Convert async tests to sync where no awaits needed - FakeTimeProvider tests are synchronous by design - Add StyleCop pragma for test file formatting

Copilot AI review requested due to automatic review settings November 29, 2025 04:53

Copilot started reviewing on behalf of michaelstonis November 29, 2025 04:53 View session

Copilot finished reviewing on behalf of michaelstonis November 29, 2025 04:55

Copilot AI reviewed Nov 29, 2025

View reviewed changes

michaelstonis merged commit f51ce8e into main Nov 29, 2025
1 check passed

michaelstonis deleted the fix/flaky-tests branch November 29, 2025 05:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhances automated workflows and test reliability#1

Enhances automated workflows and test reliability#1
michaelstonis merged 11 commits intomainfrom
fix/flaky-tests

michaelstonis commented Nov 29, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants



	// Update synchronization variables after previous assertion, before triggering the next action

		expectedCount = 4;
		emitTcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);



	// Move expectedCount and emitTcs update after previous await, before next emission

	var count = Interlocked.Increment(ref transformCount);
	Interlocked.Increment(ref transformCount);

Conversation

michaelstonis commented Nov 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants