Skip to content

Commit

Permalink
fix(fdc3) - Attempt to fix flaky tests: waiting for StartRequests to …
Browse files Browse the repository at this point in the history 
…finish before raising the intent
  • Loading branch information
@lilla28
lilla28 committed Mar 18, 2024
1 parent 7c86701 commit ec0b52b
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 26 deletions.
49 changes: 44 additions & 5 deletions src/fdc3/dotnet/DesktopAgent/src/DesktopAgent/Fdc3DesktopAgent.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ internal class Fdc3DesktopAgent : IFdc3DesktopAgentBridge
private readonly IModuleLoader _moduleLoader;
private readonly ConcurrentDictionary<Guid, Fdc3App> _runningModules = new();
private readonly ConcurrentDictionary<Guid, RaisedIntentRequestHandler> _raisedIntentResolutions = new();
private readonly ConcurrentDictionary<StartRequest, TaskCompletionSource<IModuleInstance>> _pendingStartRequests = new();
private IAsyncDisposable? _subscription;

public Fdc3DesktopAgent(
Expand Down Expand Up @@ -97,6 +98,13 @@ public async Task StopAsync(CancellationToken cancellationToken)
_runningModules.Clear();
await _subscription.DisposeAsync();
}

foreach (var pendingStartRequest in _pendingStartRequests)
{
pendingStartRequest.Value.TrySetCanceled();
}

_pendingStartRequests.Clear();
}

public bool FindChannel(string channelId, ChannelType channelType)
Expand Down Expand Up @@ -461,13 +469,33 @@ private async ValueTask<RaiseIntentResult<RaiseIntentResponse>> RaiseIntentToApp
try
{
var fdc3InstanceId = Guid.NewGuid();
var moduleInstance = await _moduleLoader.StartModule(
new StartRequest(
targetAppMetadata.AppId, //TODO: possible remove some identifier like @"fdc3."
var startRequest = new StartRequest(
targetAppMetadata.AppId, //TODO: possible remove some identifier like @"fdc3."
new List<KeyValuePair<string, string>>()
{
{ new(Fdc3StartupParameters.Fdc3InstanceId, fdc3InstanceId.ToString()) }
})) ?? throw ThrowHelper.TargetInstanceUnavailable();
});

var taskCompletionSource = new TaskCompletionSource<IModuleInstance>();

if (_pendingStartRequests.TryAdd(startRequest, taskCompletionSource))
{
var moduleInstance = await _moduleLoader.StartModule(startRequest);

if (moduleInstance == null)
{
var exception = ThrowHelper.TargetInstanceUnavailable();

if (!_pendingStartRequests.TryRemove(startRequest, out _))
{
_logger.LogWarning($"Could not remove {nameof(StartRequest)} from the pending requests. ModuleId: {startRequest.ModuleId}.");
}

taskCompletionSource.TrySetException(exception);
}

await taskCompletionSource.Task;
}

var raisedIntentMessageId = StoreRaisedIntentForTarget(messageId, fdc3InstanceId.ToString(), intent, context, sourceFdc3InstanceId);

Expand Down Expand Up @@ -496,7 +524,7 @@ private async ValueTask<RaiseIntentResult<RaiseIntentResponse>> RaiseIntentToApp

return new()
{
Response = RaiseIntentResponse.Failure(exception.Message),
Response = RaiseIntentResponse.Failure(exception.ToString()),
};
}
}
Expand Down Expand Up @@ -698,10 +726,16 @@ private Task RemoveModuleAsync(IModuleInstance instance)
try
{
var fdc3InstanceId = GetFdc3InstanceId(instance);

if (!_runningModules.TryRemove(new(fdc3InstanceId), out _))
{
_logger.LogError($"Could not remove the closed window with instanceId: {fdc3InstanceId}.");
}

if (_pendingStartRequests.TryRemove(instance.StartRequest, out var taskCompletionSource))
{
taskCompletionSource.SetException(ThrowHelper.TargetInstanceUnavailable());
}
}
catch (Fdc3DesktopAgentException exception)
{
Expand All @@ -717,6 +751,11 @@ private async Task AddOrUpdateModuleAsync(IModuleInstance instance)
try
{
fdc3App = await _appDirectory.GetApp(instance.Manifest.Id);

if (_pendingStartRequests.TryRemove(instance.StartRequest, out var taskCompletionSource))
{
taskCompletionSource.SetResult(instance);
}
}
catch (AppNotFoundException)
{
Expand Down
12 changes: 11 additions & 1 deletion src/fdc3/dotnet/DesktopAgent/tests/DesktopAgent.Tests/Fdc3DesktopAgentTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

namespace MorganStanley.ComposeUI.Fdc3.DesktopAgent.Tests;

public class Fdc3DesktopAgentTests
public class Fdc3DesktopAgentTests : IAsyncLifetime
{
private readonly MockModuleLoader _mockModuleLoader = new();
private readonly IAppDirectory _appDirectory = new AppDirectory.AppDirectory(
Expand Down Expand Up @@ -593,4 +593,14 @@ public async Task RaiseIntent_returns_one_running_app()
result!.RaiseIntentResolutionMessages!.Should().HaveCount(1);
result!.RaiseIntentResolutionMessages!.First().TargetModuleInstanceId.Should().Be(targetFdc3InstanceId);
}

public async Task InitializeAsync()
{
await _fdc3.StartAsync(CancellationToken.None);
}

public async Task DisposeAsync()
{
await _fdc3.StopAsync(CancellationToken.None);
}
}
35 changes: 15 additions & 20 deletions .../DesktopAgent.Tests/Infrastructure/Internal/Fdc3DesktopAgentMessageRouterService.Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

namespace MorganStanley.ComposeUI.Fdc3.DesktopAgent.Tests.Infrastructure.Internal;

public class Fdc3DesktopAgentMessageRouterServiceTests
public class Fdc3DesktopAgentMessageRouterServiceTests : IAsyncLifetime
{
private readonly Mock<IMessageRouter> _mockMessageRouter = new();
private readonly MockModuleLoader _mockModuleLoader = new();
Expand Down Expand Up @@ -78,7 +78,6 @@ public async Task RaiseIntent_returns_one_app_by_AppIdentifier()

var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext());
result.Should().NotBeNull();

result!.AppMetadata.Should().HaveCount(1);
result!.AppMetadata!.First()!.AppId.Should().Be("appId4");
result!.AppMetadata!.First()!.InstanceId.Should().NotBeNull();
Expand All @@ -88,7 +87,6 @@ public async Task RaiseIntent_returns_one_app_by_AppIdentifier()
public async Task RaiseIntent_fails_by_request_delivery_error()
{
var result = await _fdc3.HandleRaiseIntent(null, new MessageContext());

result.Should().NotBeNull();
result!.Error.Should().Be(ResolveError.IntentDeliveryFailed);
}
Expand All @@ -106,9 +104,7 @@ public async Task RaiseIntent_returns_one_app_by_Context()
};

var result = await _fdc3.HandleRaiseIntent(request, new MessageContext());

result.Should().NotBeNull();

result!.AppMetadata.Should().HaveCount(1);
result!.AppMetadata!.First()!.AppId.Should().Be("appId4");
result!.AppMetadata!.First()!.InstanceId.Should().NotBeNull();
Expand All @@ -133,9 +129,7 @@ public async Task RaiseIntent_returns_one_app_by_AppIdentifier_and_saves_context
};

var result = await _fdc3.HandleRaiseIntent(request, new MessageContext());

result.Should().NotBeNull();

result!.AppMetadata.Should().HaveCount(1);
result!.AppMetadata!.First()!.AppId.Should().Be("appId4");
result!.AppMetadata!.First()!.InstanceId.Should().Be(targetFdc3InstanceId);
Expand Down Expand Up @@ -169,7 +163,6 @@ public async Task RaiseIntent_returns_one_app_by_AppIdentifier_and_publishes_con

var addIntentListenerResult = await _fdc3.HandleAddIntentListener(addIntentListenerRequest, new MessageContext());
addIntentListenerResult.Should().NotBeNull();

addIntentListenerResult!.Stored.Should().BeTrue();

var request = new RaiseIntentRequest()
Expand All @@ -184,7 +177,6 @@ public async Task RaiseIntent_returns_one_app_by_AppIdentifier_and_publishes_con

var result = await _fdc3.HandleRaiseIntent(request, new MessageContext());
result.Should().NotBeNull();

result!.AppMetadata.Should().HaveCount(1);
result!.AppMetadata!.First()!.AppId.Should().Be("appId4");
result!.AppMetadata!.First()!.InstanceId.Should().Be(targetFdc3InstanceId);
Expand All @@ -209,7 +201,6 @@ public async Task RaiseIntent_returns_first_app_by_Context()

var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext());
result.Should().NotBeNull();

result!.AppMetadata.Should().HaveCount(1);
result!.AppMetadata!.First().AppId.Should().Be("appId4");
}
Expand All @@ -230,7 +221,6 @@ public async Task RaiseIntent_returns_first_app_by_Context_if_fdc3_nothing()

var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext());
result.Should().NotBeNull();

result!.AppMetadata.Should().HaveCount(1);
result!.AppMetadata!.First().AppId.Should().Be("appId4");
}
Expand Down Expand Up @@ -266,7 +256,6 @@ public async Task RaiseIntent_fails_as_no_apps_found_by_Context()
};

var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext());

result.Should().NotBeNull();
result!.Error.Should().Be(ResolveError.NoAppsFound);
}
Expand All @@ -284,7 +273,6 @@ public async Task RaiseIntent_fails_as_no_apps_found_by_Intent()
};

var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext());

result.Should().NotBeNull();
result!.Error.Should().Be(ResolveError.NoAppsFound);
}
Expand All @@ -302,7 +290,6 @@ public async Task RaiseIntent_fails_as_multiple_IAppIntents_found()
};

var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext());

result.Should().NotBeNull();
result!.Error.Should().Be(ResolveError.IntentDeliveryFailed);
}
Expand All @@ -320,7 +307,6 @@ public async Task RaiseIntent_fails_as_request_specifies_error()
};

var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext());

result!.Error.Should().Be("Some weird error");
}

Expand Down Expand Up @@ -616,7 +602,6 @@ public async Task GetIntentResult_succeeds_with_context()
};

var storeResult = await _fdc3.HandleStoreIntentResult(storeIntentRequest, new MessageContext());

storeResult.Should().NotBeNull();
storeResult!.Should().BeEquivalentTo(StoreIntentResultResponse.Success());

Expand Down Expand Up @@ -667,7 +652,6 @@ public async Task GetIntentResult_succeeds_with_channel()
};

var storeResult = await _fdc3.HandleStoreIntentResult(storeIntentRequest, new MessageContext());

storeResult.Should().NotBeNull();
storeResult!.Should().BeEquivalentTo(StoreIntentResultResponse.Success());

Expand All @@ -687,8 +671,8 @@ public async Task GetIntentResult_succeeds_with_channel()
public async Task GetIntentResult_succeeds_with_voidResult()
{
await _fdc3.StartAsync(CancellationToken.None);
var originFdc3InstanceId = Guid.NewGuid().ToString();

var originFdc3InstanceId = Guid.NewGuid().ToString();
var target = await _mockModuleLoader.Object.StartModule(new("appId4"));
var targetFdc3InstanceId = Fdc3InstanceIdRetriever.Get(target);

Expand Down Expand Up @@ -748,6 +732,7 @@ public async Task AddIntentListener_fails_due_missing_id()
Fdc3InstanceId = Guid.NewGuid().ToString(),
State = SubscribeState.Unsubscribe
};

var result = await _fdc3.HandleAddIntentListener(request, new());
result.Should().NotBeNull();
result!.Should().BeEquivalentTo(IntentListenerResponse.Failure(Fdc3DesktopAgentErrors.MissingId));
Expand Down Expand Up @@ -885,8 +870,8 @@ public async Task AddIntentListener_unsubscribes()
public async Task FindIntent_edge_case_tests(FindIntentTestCase testCase)
{
var request = testCase.Request;
var result = await _fdc3.HandleFindIntent(request, new MessageContext());

var result = await _fdc3.HandleFindIntent(request, new MessageContext());
result.Should().NotBeNull();

if (testCase.ExpectedAppCount > 0)
Expand All @@ -902,8 +887,8 @@ public async Task FindIntent_edge_case_tests(FindIntentTestCase testCase)
public async Task FindIntentsByContext_edge_case_tests(FindIntentsByContextTestCase testCase)
{
var request = testCase.Request;
var result = await _fdc3.HandleFindIntentsByContext(request, new MessageContext());

var result = await _fdc3.HandleFindIntentsByContext(request, new MessageContext());
result.Should().NotBeNull();

if (testCase.ExpectedAppIntentsCount > 0)
Expand All @@ -914,6 +899,16 @@ public async Task FindIntentsByContext_edge_case_tests(FindIntentsByContextTestC
result!.Should().BeEquivalentTo(testCase.ExpectedResponse);
}

public async Task InitializeAsync()
{
await _fdc3.StartAsync(CancellationToken.None);
}

public async Task DisposeAsync()
{
await _fdc3.StopAsync(CancellationToken.None);
}

public class FindIntentsByContextTheoryData : TheoryData
{
public FindIntentsByContextTheoryData()
Expand Down

0 comments on commit ec0b52b

Please sign in to comment.