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 <noreply@anthropic.com>
This commit is contained in:
Sipke Schoorstra 2026-08-20 23:30:38 +02:00 committed by GitHub
parent 74fc891350
commit a02ebff129
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 33 additions and 11 deletions

View file

@ -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<ReceivedEventPayload>(JsonSerializer.Serialize(receivedPayload), CaseInsensitive);
Assert.Equal("Shipped", payload?.Status);
}
private record ReceivedEventPayload(string? Status);
private async Task<WorkflowInstance> GetSingleWorkflowInstanceAsync(string definitionId, string correlationId, int timeoutMs = 5000)
{
var tcs = new TaskCompletionSource<WorkflowInstance>();
@ -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

View file

@ -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<IShellRegistry>();
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<IApiSerializer>();
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)
{

View file

@ -0,0 +1,19 @@
using System.Runtime.CompilerServices;
namespace Elsa.Shells.Api.Tests;
/// <summary>
/// <see cref="EndpointSecurityOptions.SecurityIsEnabled"/> is process-global mutable state that FastEndpoints reads
/// once per host, while <c>UseFastEndpoints()</c> 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
/// <c>true</c> between another class's write and its <c>UseFastEndpoints()</c> call produces endpoints carrying
/// authorization metadata in a pipeline that has no <c>UseAuthorization</c>, 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.
/// </summary>
internal static class TestSecurityDefaults
{
[ModuleInitializer]
internal static void DisableEndpointSecurity() => EndpointSecurityOptions.SecurityIsEnabled = false;
}