From 063f829d6fe0f734f4c06533b5ea7146da79e233 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 1 Nov 2025 22:21:07 +0000 Subject: [PATCH 1/4] Initial plan From 99a967f604bd4c05d1d4bb39752802c7ebe33743 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 1 Nov 2025 22:36:20 +0000 Subject: [PATCH 2/4] Make state changes on IReportingStep and IReportingTask idempotent Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../Pipelines/PipelineActivityReporter.cs | 21 +- .../PipelineActivityReporterTests.cs | 224 +++++++++++++++++- 2 files changed, 231 insertions(+), 14 deletions(-) diff --git a/src/Aspire.Hosting/Pipelines/PipelineActivityReporter.cs b/src/Aspire.Hosting/Pipelines/PipelineActivityReporter.cs index 654daa4f576..b705d0e3311 100644 --- a/src/Aspire.Hosting/Pipelines/PipelineActivityReporter.cs +++ b/src/Aspire.Hosting/Pipelines/PipelineActivityReporter.cs @@ -99,10 +99,17 @@ public async Task CompleteStepAsync(ReportingStep step, string completionText, C { lock (step) { - // Prevent double completion if the step is already complete + // If the step is already in a terminal state if (step.CompletionState != CompletionState.InProgress) { - throw new InvalidOperationException($"Cannot complete step '{step.Id}' with state '{step.CompletionState}'. Only 'InProgress' steps can be completed."); + // If trying to complete with the same state, this is a noop (idempotent) + if (step.CompletionState == completionState) + { + return; + } + + // If trying to transition from a terminal state to a different terminal state, this is a coding bug + throw new InvalidOperationException($"Cannot complete step '{step.Id}' with state '{completionState}'. Step is already in terminal state '{step.CompletionState}'."); } step.CompletionState = completionState; @@ -196,9 +203,17 @@ public async Task CompleteTaskAsync(ReportingTask task, CompletionState completi throw new InvalidOperationException($"Parent step with ID '{task.StepId}' does not exist."); } + // If the task is already in a terminal state if (task.CompletionState != CompletionState.InProgress) { - throw new InvalidOperationException($"Cannot complete task '{task.Id}' with state '{task.CompletionState}'. Only 'InProgress' tasks can be completed."); + // If trying to complete with the same state, this is a noop (idempotent) + if (task.CompletionState == completionState) + { + return; + } + + // If trying to transition from a terminal state to a different terminal state, this is a coding bug + throw new InvalidOperationException($"Cannot complete task '{task.Id}' with state '{completionState}'. Task is already in terminal state '{task.CompletionState}'."); } lock (parentStep) diff --git a/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs b/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs index 7ae5f85e5bc..ba6398bc4db 100644 --- a/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs +++ b/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs @@ -376,7 +376,7 @@ public async Task CompleteTaskAsync_WithNullCompletionMessage_SetsEmptyString() } [Fact] - public async Task CompleteTaskAsync_ThrowsWhenTaskAlreadyCompleted() + public async Task CompleteTaskAsync_IdempotentWhenCompletedWithSameState() { // Arrange var reporter = CreatePublishingReporter(); @@ -387,16 +387,41 @@ public async Task CompleteTaskAsync_ThrowsWhenTaskAlreadyCompleted() // Complete the task first time await task.CompleteAsync(null, cancellationToken: CancellationToken.None); - // Act & Assert - Try to complete the same task again + // Clear activities + while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + + // Act - Try to complete the same task again with the same state (should be idempotent, no exception) + await task.CompleteAsync(null, cancellationToken: CancellationToken.None); + + // Assert - No new activity should be emitted (noop) + Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + + var taskInternal = Assert.IsType(task); + Assert.Equal(CompletionState.Completed, taskInternal.CompletionState); + } + + [Fact] + public async Task CompleteTaskAsync_ThrowsWhenCompletedWithDifferentState() + { + // Arrange + var reporter = CreatePublishingReporter(); + + var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None); + var task = await step.CreateTaskAsync("Test Task", CancellationToken.None); + + // Complete the task first time successfully + await task.CompleteAsync(null, CompletionState.Completed, cancellationToken: CancellationToken.None); + + // Act & Assert - Try to complete with a different state (should throw) var exception = await Assert.ThrowsAsync( - () => task.CompleteAsync(null, cancellationToken: CancellationToken.None)); + () => task.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None)); var taskInternal = Assert.IsType(task); - Assert.Contains($"Cannot complete task '{taskInternal.Id}' with state 'Completed'. Only 'InProgress' tasks can be completed.", exception.Message); + Assert.Contains($"Cannot complete task '{taskInternal.Id}' with state 'CompletedWithError'. Task is already in terminal state 'Completed'.", exception.Message); } [Fact] - public async Task CompleteStepAsync_ThrowsWhenStepAlreadyCompleted() + public async Task CompleteStepAsync_IdempotentWhenCompletedWithSameState() { // Arrange var reporter = CreatePublishingReporter(); @@ -406,12 +431,37 @@ public async Task CompleteStepAsync_ThrowsWhenStepAlreadyCompleted() // Complete the step first time await step.CompleteAsync("Complete", cancellationToken: CancellationToken.None); - // Act & Assert - Try to complete the same step again + // Clear activities + while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + + // Act - Try to complete the same step again with the same state (should be idempotent, no exception) + await step.CompleteAsync("Complete again", cancellationToken: CancellationToken.None); + + // Assert - No new activity should be emitted (noop) + Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + + var stepInternal = Assert.IsType(step); + Assert.Equal(CompletionState.Completed, stepInternal.CompletionState); + Assert.Equal("Complete", stepInternal.CompletionText); // Original completion text is retained + } + + [Fact] + public async Task CompleteStepAsync_ThrowsWhenCompletedWithDifferentState() + { + // Arrange + var reporter = CreatePublishingReporter(); + + var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None); + + // Complete the step first time successfully + await step.CompleteAsync("Complete", CompletionState.Completed, cancellationToken: CancellationToken.None); + + // Act & Assert - Try to complete with a different state (should throw) var exception = await Assert.ThrowsAsync( - () => step.CompleteAsync("Complete again", cancellationToken: CancellationToken.None)); + () => step.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None)); var stepInternal = Assert.IsType(step); - Assert.Contains($"Cannot complete step '{stepInternal.Id}' with state 'Completed'. Only 'InProgress' steps can be completed.", exception.Message); + Assert.Contains($"Cannot complete step '{stepInternal.Id}' with state 'CompletedWithError'. Step is already in terminal state 'Completed'.", exception.Message); } [Fact] @@ -436,10 +486,13 @@ public async Task CompleteStepAsync_KeepsStepInDictionaryForAggregation() var taskInternal = Assert.IsType(task); Assert.Contains($"Cannot update task '{taskInternal.Id}' because its parent step", updateException.Message); - // For CompleteTaskAsync, it will first check if the task is already completed, so we expect that error instead + // For CompleteTaskAsync, since task is already completed, attempting to complete with same state should be idempotent + await task.CompleteAsync(null, cancellationToken: CancellationToken.None); // Should not throw + + // Attempting to complete with a different state should throw var completeException = await Assert.ThrowsAsync( - () => task.CompleteAsync(null, cancellationToken: CancellationToken.None)); - Assert.Contains($"Cannot complete task '{taskInternal.Id}' with state 'Completed'. Only 'InProgress' tasks can be completed.", completeException.Message); + () => task.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None)); + Assert.Contains($"Cannot complete task '{taskInternal.Id}' with state 'CompletedWithError'. Task is already in terminal state 'Completed'.", completeException.Message); // Creating new tasks for the completed step should also fail because the step is complete var createException = await Assert.ThrowsAsync( @@ -913,6 +966,155 @@ public async Task CompleteTaskAsync_WithMarkdownCompletionMessage_PreservesMarkd Assert.Equal(markdownCompletionMessage, activity.Data.CompletionMessage); } + [Fact] + public async Task CompleteTaskAsync_IdempotentWithWarning() + { + // Arrange + var reporter = CreatePublishingReporter(); + + var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None); + var task = await step.CreateTaskAsync("Test Task", CancellationToken.None); + + // Complete the task with warning + await task.CompleteAsync("Warning message", CompletionState.CompletedWithWarning, CancellationToken.None); + + // Clear activities + while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + + // Act - Try to complete again with same state (should be idempotent) + await task.CompleteAsync("Different warning message", CompletionState.CompletedWithWarning, CancellationToken.None); + + // Assert - No new activity should be emitted + Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + + var taskInternal = Assert.IsType(task); + Assert.Equal(CompletionState.CompletedWithWarning, taskInternal.CompletionState); + } + + [Fact] + public async Task CompleteTaskAsync_IdempotentWithError() + { + // Arrange + var reporter = CreatePublishingReporter(); + + var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None); + var task = await step.CreateTaskAsync("Test Task", CancellationToken.None); + + // Complete the task with error + await task.CompleteAsync("Error message", CompletionState.CompletedWithError, CancellationToken.None); + + // Clear activities + while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + + // Act - Try to complete again with same state (should be idempotent) + await task.CompleteAsync("Different error message", CompletionState.CompletedWithError, CancellationToken.None); + + // Assert - No new activity should be emitted + Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + + var taskInternal = Assert.IsType(task); + Assert.Equal(CompletionState.CompletedWithError, taskInternal.CompletionState); + } + + [Fact] + public async Task CompleteStepAsync_IdempotentWithWarning() + { + // Arrange + var reporter = CreatePublishingReporter(); + + var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None); + + // Complete the step with warning + await step.CompleteAsync("Warning", CompletionState.CompletedWithWarning, CancellationToken.None); + + // Clear activities + while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + + // Act - Try to complete again with same state (should be idempotent) + await step.CompleteAsync("Different warning", CompletionState.CompletedWithWarning, CancellationToken.None); + + // Assert - No new activity should be emitted + Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + + var stepInternal = Assert.IsType(step); + Assert.Equal(CompletionState.CompletedWithWarning, stepInternal.CompletionState); + } + + [Fact] + public async Task CompleteStepAsync_IdempotentWithError() + { + // Arrange + var reporter = CreatePublishingReporter(); + + var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None); + + // Complete the step with error + await step.CompleteAsync("Error", CompletionState.CompletedWithError, CancellationToken.None); + + // Clear activities + while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + + // Act - Try to complete again with same state (should be idempotent) + await step.CompleteAsync("Different error", CompletionState.CompletedWithError, CancellationToken.None); + + // Assert - No new activity should be emitted + Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + + var stepInternal = Assert.IsType(step); + Assert.Equal(CompletionState.CompletedWithError, stepInternal.CompletionState); + } + + [Theory] + [InlineData(CompletionState.Completed, CompletionState.CompletedWithWarning)] + [InlineData(CompletionState.Completed, CompletionState.CompletedWithError)] + [InlineData(CompletionState.CompletedWithWarning, CompletionState.Completed)] + [InlineData(CompletionState.CompletedWithWarning, CompletionState.CompletedWithError)] + [InlineData(CompletionState.CompletedWithError, CompletionState.Completed)] + [InlineData(CompletionState.CompletedWithError, CompletionState.CompletedWithWarning)] + public async Task CompleteTaskAsync_ThrowsWhenTransitioningBetweenTerminalStates(CompletionState firstState, CompletionState secondState) + { + // Arrange + var reporter = CreatePublishingReporter(); + + var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None); + var task = await step.CreateTaskAsync("Test Task", CancellationToken.None); + + // Complete the task with first state + await task.CompleteAsync("First completion", firstState, CancellationToken.None); + + // Act & Assert - Try to complete with different state (should throw) + var exception = await Assert.ThrowsAsync( + () => task.CompleteAsync("Second completion", secondState, CancellationToken.None)); + + var taskInternal = Assert.IsType(task); + Assert.Contains($"Cannot complete task '{taskInternal.Id}' with state '{secondState}'. Task is already in terminal state '{firstState}'.", exception.Message); + } + + [Theory] + [InlineData(CompletionState.Completed, CompletionState.CompletedWithWarning)] + [InlineData(CompletionState.Completed, CompletionState.CompletedWithError)] + [InlineData(CompletionState.CompletedWithWarning, CompletionState.Completed)] + [InlineData(CompletionState.CompletedWithWarning, CompletionState.CompletedWithError)] + [InlineData(CompletionState.CompletedWithError, CompletionState.Completed)] + [InlineData(CompletionState.CompletedWithError, CompletionState.CompletedWithWarning)] + public async Task CompleteStepAsync_ThrowsWhenTransitioningBetweenTerminalStates(CompletionState firstState, CompletionState secondState) + { + // Arrange + var reporter = CreatePublishingReporter(); + + var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None); + + // Complete the step with first state + await step.CompleteAsync("First completion", firstState, CancellationToken.None); + + // Act & Assert - Try to complete with different state (should throw) + var exception = await Assert.ThrowsAsync( + () => step.CompleteAsync("Second completion", secondState, CancellationToken.None)); + + var stepInternal = Assert.IsType(step); + Assert.Contains($"Cannot complete step '{stepInternal.Id}' with state '{secondState}'. Step is already in terminal state '{firstState}'.", exception.Message); + } + private PipelineActivityReporter CreatePublishingReporter() { return new PipelineActivityReporter(_interactionService, NullLogger.Instance); From c9582ef0684348754012693fff5e0122dfc30e2b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 1 Nov 2025 22:56:01 +0000 Subject: [PATCH 3/4] Make ALL terminal state transitions idempotent (noop) Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../Pipelines/PipelineActivityReporter.cs | 22 ++---- .../PipelineActivityReporterTests.cs | 69 ++++++++++++------- 2 files changed, 47 insertions(+), 44 deletions(-) diff --git a/src/Aspire.Hosting/Pipelines/PipelineActivityReporter.cs b/src/Aspire.Hosting/Pipelines/PipelineActivityReporter.cs index b705d0e3311..72af3b03c2a 100644 --- a/src/Aspire.Hosting/Pipelines/PipelineActivityReporter.cs +++ b/src/Aspire.Hosting/Pipelines/PipelineActivityReporter.cs @@ -99,17 +99,10 @@ public async Task CompleteStepAsync(ReportingStep step, string completionText, C { lock (step) { - // If the step is already in a terminal state + // If the step is already in a terminal state, this is a noop (idempotent) if (step.CompletionState != CompletionState.InProgress) { - // If trying to complete with the same state, this is a noop (idempotent) - if (step.CompletionState == completionState) - { - return; - } - - // If trying to transition from a terminal state to a different terminal state, this is a coding bug - throw new InvalidOperationException($"Cannot complete step '{step.Id}' with state '{completionState}'. Step is already in terminal state '{step.CompletionState}'."); + return; } step.CompletionState = completionState; @@ -203,17 +196,10 @@ public async Task CompleteTaskAsync(ReportingTask task, CompletionState completi throw new InvalidOperationException($"Parent step with ID '{task.StepId}' does not exist."); } - // If the task is already in a terminal state + // If the task is already in a terminal state, this is a noop (idempotent) if (task.CompletionState != CompletionState.InProgress) { - // If trying to complete with the same state, this is a noop (idempotent) - if (task.CompletionState == completionState) - { - return; - } - - // If trying to transition from a terminal state to a different terminal state, this is a coding bug - throw new InvalidOperationException($"Cannot complete task '{task.Id}' with state '{completionState}'. Task is already in terminal state '{task.CompletionState}'."); + return; } lock (parentStep) diff --git a/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs b/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs index ba6398bc4db..4be36fc1b13 100644 --- a/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs +++ b/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs @@ -401,7 +401,7 @@ public async Task CompleteTaskAsync_IdempotentWhenCompletedWithSameState() } [Fact] - public async Task CompleteTaskAsync_ThrowsWhenCompletedWithDifferentState() + public async Task CompleteTaskAsync_IdempotentWhenCompletedWithDifferentState() { // Arrange var reporter = CreatePublishingReporter(); @@ -412,12 +412,17 @@ public async Task CompleteTaskAsync_ThrowsWhenCompletedWithDifferentState() // Complete the task first time successfully await task.CompleteAsync(null, CompletionState.Completed, cancellationToken: CancellationToken.None); - // Act & Assert - Try to complete with a different state (should throw) - var exception = await Assert.ThrowsAsync( - () => task.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None)); + // Clear activities + while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + + // Act - Try to complete with a different state (should be idempotent, no exception) + await task.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None); + // Assert - No new activity should be emitted (noop) + Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + var taskInternal = Assert.IsType(task); - Assert.Contains($"Cannot complete task '{taskInternal.Id}' with state 'CompletedWithError'. Task is already in terminal state 'Completed'.", exception.Message); + Assert.Equal(CompletionState.Completed, taskInternal.CompletionState); // Original state is retained } [Fact] @@ -446,7 +451,7 @@ public async Task CompleteStepAsync_IdempotentWhenCompletedWithSameState() } [Fact] - public async Task CompleteStepAsync_ThrowsWhenCompletedWithDifferentState() + public async Task CompleteStepAsync_IdempotentWhenCompletedWithDifferentState() { // Arrange var reporter = CreatePublishingReporter(); @@ -456,12 +461,18 @@ public async Task CompleteStepAsync_ThrowsWhenCompletedWithDifferentState() // Complete the step first time successfully await step.CompleteAsync("Complete", CompletionState.Completed, cancellationToken: CancellationToken.None); - // Act & Assert - Try to complete with a different state (should throw) - var exception = await Assert.ThrowsAsync( - () => step.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None)); + // Clear activities + while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + // Act - Try to complete with a different state (should be idempotent, no exception) + await step.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None); + + // Assert - No new activity should be emitted (noop) + Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + var stepInternal = Assert.IsType(step); - Assert.Contains($"Cannot complete step '{stepInternal.Id}' with state 'CompletedWithError'. Step is already in terminal state 'Completed'.", exception.Message); + Assert.Equal(CompletionState.Completed, stepInternal.CompletionState); // Original state is retained + Assert.Equal("Complete", stepInternal.CompletionText); // Original completion text is retained } [Fact] @@ -486,13 +497,9 @@ public async Task CompleteStepAsync_KeepsStepInDictionaryForAggregation() var taskInternal = Assert.IsType(task); Assert.Contains($"Cannot update task '{taskInternal.Id}' because its parent step", updateException.Message); - // For CompleteTaskAsync, since task is already completed, attempting to complete with same state should be idempotent + // For CompleteTaskAsync, since task is already completed, attempting to complete with same or different state should be idempotent await task.CompleteAsync(null, cancellationToken: CancellationToken.None); // Should not throw - - // Attempting to complete with a different state should throw - var completeException = await Assert.ThrowsAsync( - () => task.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None)); - Assert.Contains($"Cannot complete task '{taskInternal.Id}' with state 'CompletedWithError'. Task is already in terminal state 'Completed'.", completeException.Message); + await task.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None); // Should also not throw (noop) // Creating new tasks for the completed step should also fail because the step is complete var createException = await Assert.ThrowsAsync( @@ -1071,7 +1078,7 @@ public async Task CompleteStepAsync_IdempotentWithError() [InlineData(CompletionState.CompletedWithWarning, CompletionState.CompletedWithError)] [InlineData(CompletionState.CompletedWithError, CompletionState.Completed)] [InlineData(CompletionState.CompletedWithError, CompletionState.CompletedWithWarning)] - public async Task CompleteTaskAsync_ThrowsWhenTransitioningBetweenTerminalStates(CompletionState firstState, CompletionState secondState) + public async Task CompleteTaskAsync_IdempotentWhenTransitioningBetweenTerminalStates(CompletionState firstState, CompletionState secondState) { // Arrange var reporter = CreatePublishingReporter(); @@ -1082,12 +1089,17 @@ public async Task CompleteTaskAsync_ThrowsWhenTransitioningBetweenTerminalStates // Complete the task with first state await task.CompleteAsync("First completion", firstState, CancellationToken.None); - // Act & Assert - Try to complete with different state (should throw) - var exception = await Assert.ThrowsAsync( - () => task.CompleteAsync("Second completion", secondState, CancellationToken.None)); + // Clear activities + while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + // Act - Try to complete with different state (should be idempotent, no exception) + await task.CompleteAsync("Second completion", secondState, CancellationToken.None); + + // Assert - No new activity should be emitted (noop) + Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + var taskInternal = Assert.IsType(task); - Assert.Contains($"Cannot complete task '{taskInternal.Id}' with state '{secondState}'. Task is already in terminal state '{firstState}'.", exception.Message); + Assert.Equal(firstState, taskInternal.CompletionState); // Original state is retained } [Theory] @@ -1097,7 +1109,7 @@ public async Task CompleteTaskAsync_ThrowsWhenTransitioningBetweenTerminalStates [InlineData(CompletionState.CompletedWithWarning, CompletionState.CompletedWithError)] [InlineData(CompletionState.CompletedWithError, CompletionState.Completed)] [InlineData(CompletionState.CompletedWithError, CompletionState.CompletedWithWarning)] - public async Task CompleteStepAsync_ThrowsWhenTransitioningBetweenTerminalStates(CompletionState firstState, CompletionState secondState) + public async Task CompleteStepAsync_IdempotentWhenTransitioningBetweenTerminalStates(CompletionState firstState, CompletionState secondState) { // Arrange var reporter = CreatePublishingReporter(); @@ -1107,12 +1119,17 @@ public async Task CompleteStepAsync_ThrowsWhenTransitioningBetweenTerminalStates // Complete the step with first state await step.CompleteAsync("First completion", firstState, CancellationToken.None); - // Act & Assert - Try to complete with different state (should throw) - var exception = await Assert.ThrowsAsync( - () => step.CompleteAsync("Second completion", secondState, CancellationToken.None)); + // Clear activities + while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + + // Act - Try to complete with different state (should be idempotent, no exception) + await step.CompleteAsync("Second completion", secondState, CancellationToken.None); + // Assert - No new activity should be emitted (noop) + Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + var stepInternal = Assert.IsType(step); - Assert.Contains($"Cannot complete step '{stepInternal.Id}' with state '{secondState}'. Step is already in terminal state '{firstState}'.", exception.Message); + Assert.Equal(firstState, stepInternal.CompletionState); // Original state is retained } private PipelineActivityReporter CreatePublishingReporter() From dea6fa81582cc08da4424556d8d7d45a2009d382 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 2 Nov 2025 03:12:17 +0000 Subject: [PATCH 4/4] Extract helper methods for clearing activities and asserting no activities Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../PipelineActivityReporterTests.cs | 60 +++++++++++-------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs b/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs index 4be36fc1b13..08390719213 100644 --- a/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs +++ b/tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs @@ -388,13 +388,13 @@ public async Task CompleteTaskAsync_IdempotentWhenCompletedWithSameState() await task.CompleteAsync(null, cancellationToken: CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Try to complete the same task again with the same state (should be idempotent, no exception) await task.CompleteAsync(null, cancellationToken: CancellationToken.None); // Assert - No new activity should be emitted (noop) - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); var taskInternal = Assert.IsType(task); Assert.Equal(CompletionState.Completed, taskInternal.CompletionState); @@ -413,13 +413,13 @@ public async Task CompleteTaskAsync_IdempotentWhenCompletedWithDifferentState() await task.CompleteAsync(null, CompletionState.Completed, cancellationToken: CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Try to complete with a different state (should be idempotent, no exception) await task.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None); // Assert - No new activity should be emitted (noop) - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); var taskInternal = Assert.IsType(task); Assert.Equal(CompletionState.Completed, taskInternal.CompletionState); // Original state is retained @@ -437,13 +437,13 @@ public async Task CompleteStepAsync_IdempotentWhenCompletedWithSameState() await step.CompleteAsync("Complete", cancellationToken: CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Try to complete the same step again with the same state (should be idempotent, no exception) await step.CompleteAsync("Complete again", cancellationToken: CancellationToken.None); // Assert - No new activity should be emitted (noop) - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); var stepInternal = Assert.IsType(step); Assert.Equal(CompletionState.Completed, stepInternal.CompletionState); @@ -462,13 +462,13 @@ public async Task CompleteStepAsync_IdempotentWhenCompletedWithDifferentState() await step.CompleteAsync("Complete", CompletionState.Completed, cancellationToken: CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Try to complete with a different state (should be idempotent, no exception) await step.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None); // Assert - No new activity should be emitted (noop) - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); var stepInternal = Assert.IsType(step); Assert.Equal(CompletionState.Completed, stepInternal.CompletionState); // Original state is retained @@ -690,7 +690,7 @@ public async Task DisposeAsync_StepWithCompletedTasks_CompletesWithSuccessState( await task2.SucceedAsync(null, CancellationToken.None); // Clear previous activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act await step.DisposeAsync(); @@ -721,13 +721,13 @@ public async Task DisposeAsync_StepAlreadyCompleted_DoesNotCompleteAgain() await step.CompleteAsync("Step completed manually", CompletionState.Completed, CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Dispose should not cause another completion await step.DisposeAsync(); // Assert - No new activities should be emitted - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); var stepInternal = Assert.IsType(step); Assert.Equal(CompletionState.Completed, stepInternal.CompletionState); Assert.Equal("Step completed manually", stepInternal.CompletionText); @@ -936,13 +936,13 @@ public async Task Log_DoesNothingWhenStepIsComplete() await step.CompleteAsync("Completed", CompletionState.Completed, CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Step is completed, so logging should be a no-op step.Log(LogLevel.Information, "Test log", enableMarkdown: false); // Assert - No new activity should be emitted - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); } [Fact] @@ -986,13 +986,13 @@ public async Task CompleteTaskAsync_IdempotentWithWarning() await task.CompleteAsync("Warning message", CompletionState.CompletedWithWarning, CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Try to complete again with same state (should be idempotent) await task.CompleteAsync("Different warning message", CompletionState.CompletedWithWarning, CancellationToken.None); // Assert - No new activity should be emitted - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); var taskInternal = Assert.IsType(task); Assert.Equal(CompletionState.CompletedWithWarning, taskInternal.CompletionState); @@ -1011,13 +1011,13 @@ public async Task CompleteTaskAsync_IdempotentWithError() await task.CompleteAsync("Error message", CompletionState.CompletedWithError, CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Try to complete again with same state (should be idempotent) await task.CompleteAsync("Different error message", CompletionState.CompletedWithError, CancellationToken.None); // Assert - No new activity should be emitted - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); var taskInternal = Assert.IsType(task); Assert.Equal(CompletionState.CompletedWithError, taskInternal.CompletionState); @@ -1035,13 +1035,13 @@ public async Task CompleteStepAsync_IdempotentWithWarning() await step.CompleteAsync("Warning", CompletionState.CompletedWithWarning, CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Try to complete again with same state (should be idempotent) await step.CompleteAsync("Different warning", CompletionState.CompletedWithWarning, CancellationToken.None); // Assert - No new activity should be emitted - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); var stepInternal = Assert.IsType(step); Assert.Equal(CompletionState.CompletedWithWarning, stepInternal.CompletionState); @@ -1059,13 +1059,13 @@ public async Task CompleteStepAsync_IdempotentWithError() await step.CompleteAsync("Error", CompletionState.CompletedWithError, CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Try to complete again with same state (should be idempotent) await step.CompleteAsync("Different error", CompletionState.CompletedWithError, CancellationToken.None); // Assert - No new activity should be emitted - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); var stepInternal = Assert.IsType(step); Assert.Equal(CompletionState.CompletedWithError, stepInternal.CompletionState); @@ -1090,13 +1090,13 @@ public async Task CompleteTaskAsync_IdempotentWhenTransitioningBetweenTerminalSt await task.CompleteAsync("First completion", firstState, CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Try to complete with different state (should be idempotent, no exception) await task.CompleteAsync("Second completion", secondState, CancellationToken.None); // Assert - No new activity should be emitted (noop) - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); var taskInternal = Assert.IsType(task); Assert.Equal(firstState, taskInternal.CompletionState); // Original state is retained @@ -1120,18 +1120,28 @@ public async Task CompleteStepAsync_IdempotentWhenTransitioningBetweenTerminalSt await step.CompleteAsync("First completion", firstState, CancellationToken.None); // Clear activities - while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + ClearActivities(reporter); // Act - Try to complete with different state (should be idempotent, no exception) await step.CompleteAsync("Second completion", secondState, CancellationToken.None); // Assert - No new activity should be emitted (noop) - Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + AssertNoActivitiesEmitted(reporter); var stepInternal = Assert.IsType(step); Assert.Equal(firstState, stepInternal.CompletionState); // Original state is retained } + private static void ClearActivities(PipelineActivityReporter reporter) + { + while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { } + } + + private static void AssertNoActivitiesEmitted(PipelineActivityReporter reporter) + { + Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _)); + } + private PipelineActivityReporter CreatePublishingReporter() { return new PipelineActivityReporter(_interactionService, NullLogger.Instance);