From a02ebff1297e774c959b85ec2bd5cb48d1a231b5 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 20 Aug 2026 23:30:38 +0200 Subject: [PATCH] test: fix two intermittent test failures (#7957) (#7965) Both tests read a value that is usually one thing and occasionally another, with a race deciding which. ReloadTests: EndpointSecurityOptions.SecurityIsEnabled is a process- global static, and ShellsApiTestBase saved/set/restored it per test method. ReloadTests and ReloadAllTests carry no [Collection], so xUnit runs them in parallel. FastEndpoints reads that global once per host while UseFastEndpoints() configures the endpoints, so when one class's DisposeAsync restores true inside another class's set-false -> UseFastEndpoints() window, that host's endpoints get authorization metadata in a pipeline with no UseAuthorization, and every request to them throws. Every test in the assembly wants security off, so set it once in a module initializer and stop mutating it per test. PublishEvent_WithPayload_TransmitsPayloadToConsumer: the payload's representation is not stable. While it is still the original CLR object its properties are PascalCase; once it has been through JsonWorkflowStateSerializer it is an ExpandoObject whose keys were camelCased by that serializer's naming policy. Which one the test sees depends on whether GetSingleWorkflowInstanceAsync returned the live in-memory instance or one read back from the store, and TryGetProperty is case-sensitive. Assert the payload's content through a DTO with PropertyNameCaseInsensitive instead of one of the two representations. Also require a terminal instance at both exits of GetSingleWorkflowInstanceAsync: it accepted any save, and an instance is saved several times over its lifetime, so it could hand a caller that asserts Finished an instance that is still running. Co-authored-by: Claude Opus 5 --- .../Primitives/Event/PublishEventTests.cs | 20 +++++++++++++------ .../ShellsApiTestBase.cs | 5 ----- .../TestSecurityDefaults.cs | 19 ++++++++++++++++++ 3 files changed, 33 insertions(+), 11 deletions(-) create mode 100644 test/unit/Elsa.Shells.Api.Tests/TestSecurityDefaults.cs diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/Primitives/Event/PublishEventTests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/Primitives/Event/PublishEventTests.cs index b1390b510..1139242b0 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/Primitives/Event/PublishEventTests.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/Primitives/Event/PublishEventTests.cs @@ -20,6 +20,7 @@ public class PublishEventTests : AppComponentTest private readonly IWorkflowInstanceStore _workflowInstanceStore; private readonly IWorkflowRuntime _workflowRuntime; private readonly WorkflowEvents _workflowEvents; + private static readonly JsonSerializerOptions CaseInsensitive = new() { PropertyNameCaseInsensitive = true }; public PublishEventTests(App app) : base(app) { @@ -72,12 +73,17 @@ public class PublishEventTests : AppComponentTest Assert.True(consumerInstance.WorkflowState.Output.TryGetValue("ReceivedPayload", out var receivedPayload), "Consumer workflow should have ReceivedPayload output"); Assert.NotNull(receivedPayload); - // Verify the payload structure and content - using var payloadDocument = JsonDocument.Parse(JsonSerializer.Serialize(receivedPayload)); - Assert.True(payloadDocument.RootElement.TryGetProperty("Status", out var status), "Received payload should contain a Status property"); - Assert.Equal("Shipped", status.GetString()); + // Verify the payload content. The payload's runtime representation is not stable: while it is still the + // original CLR object its properties are PascalCase, but once it has been through the workflow state + // serializer it is an ExpandoObject whose keys have been camelCased by that serializer's naming policy. + // Which one this test observes depends on whether the instance was read back from the store, so match + // the property name case-insensitively rather than asserting one of the two representations. + var payload = JsonSerializer.Deserialize(JsonSerializer.Serialize(receivedPayload), CaseInsensitive); + Assert.Equal("Shipped", payload?.Status); } + private record ReceivedEventPayload(string? Status); + private async Task GetSingleWorkflowInstanceAsync(string definitionId, string correlationId, int timeoutMs = 5000) { var tcs = new TaskCompletionSource(); @@ -87,9 +93,11 @@ public class PublishEventTests : AppComponentTest cts.Token.Register(() => tcs.TrySetException(new TimeoutException($"Workflow instance with DefinitionId '{definitionId}' and CorrelationId '{correlationId}' was not saved within {timeoutMs}ms"))); // Subscribe to the WorkflowInstanceSaved event + // A workflow instance is saved several times over its lifetime, so only accept a terminal one: both callers + // assert on the finished state, and an intermediate save would hand them an instance that is still running. void OnWorkflowInstanceSaved(object? sender, WorkflowInstanceSavedEventArgs args) { - if (args.WorkflowInstance.DefinitionId == definitionId && args.WorkflowInstance.CorrelationId == correlationId) + if (args.WorkflowInstance.DefinitionId == definitionId && args.WorkflowInstance.CorrelationId == correlationId && args.WorkflowInstance.Status == WorkflowStatus.Finished) { tcs.TrySetResult(args.WorkflowInstance); } @@ -106,7 +114,7 @@ public class PublishEventTests : AppComponentTest CorrelationId = correlationId }, cts.Token)).ToList(); - if (existingInstances.Any()) + if (existingInstances.Any(x => x.Status == WorkflowStatus.Finished)) return Assert.Single(existingInstances); // Wait for the event to be raised diff --git a/test/unit/Elsa.Shells.Api.Tests/ShellsApiTestBase.cs b/test/unit/Elsa.Shells.Api.Tests/ShellsApiTestBase.cs index 74cec649f..7ea4d5ad4 100644 --- a/test/unit/Elsa.Shells.Api.Tests/ShellsApiTestBase.cs +++ b/test/unit/Elsa.Shells.Api.Tests/ShellsApiTestBase.cs @@ -14,7 +14,6 @@ namespace Elsa.Shells.Api.Tests; public abstract class ShellsApiTestBase : IAsyncLifetime { private WebApplication? _app; - private bool _wasSecurityEnabled; protected IShellRegistry ShellRegistry { get; } = Substitute.For(); protected HttpClient HttpClient { get; private set; } = null!; @@ -27,9 +26,6 @@ public abstract class ShellsApiTestBase : IAsyncLifetime public async Task InitializeAsync() { - _wasSecurityEnabled = EndpointSecurityOptions.SecurityIsEnabled; - EndpointSecurityOptions.SecurityIsEnabled = false; - var apiSerializer = Substitute.For(); apiSerializer.GetOptions().Returns(JsonOptions); @@ -56,7 +52,6 @@ public abstract class ShellsApiTestBase : IAsyncLifetime public async Task DisposeAsync() { - EndpointSecurityOptions.SecurityIsEnabled = _wasSecurityEnabled; HttpClient.Dispose(); if (_app != null) { diff --git a/test/unit/Elsa.Shells.Api.Tests/TestSecurityDefaults.cs b/test/unit/Elsa.Shells.Api.Tests/TestSecurityDefaults.cs new file mode 100644 index 000000000..c927bfc23 --- /dev/null +++ b/test/unit/Elsa.Shells.Api.Tests/TestSecurityDefaults.cs @@ -0,0 +1,19 @@ +using System.Runtime.CompilerServices; + +namespace Elsa.Shells.Api.Tests; + +/// +/// is process-global mutable state that FastEndpoints reads +/// once per host, while UseFastEndpoints() configures the endpoints. xUnit runs the test classes in this +/// assembly in parallel, so a per-test save/restore of that global races: one class restoring the flag to +/// true between another class's write and its UseFastEndpoints() call produces endpoints carrying +/// authorization metadata in a pipeline that has no UseAuthorization, which fails the request with +/// "contains authorization metadata, but a middleware was not found that supports authorization". +/// +/// Every test in this assembly wants security disabled, so set it once before any test runs and never touch it again. +/// +internal static class TestSecurityDefaults +{ + [ModuleInitializer] + internal static void DisableEndpointSecurity() => EndpointSecurityOptions.SecurityIsEnabled = false; +}