Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(fdc3) - Fixing flaky DesktopAgent tests #537

Merged
merged 1 commit into from
Mar 19, 2024
Merged

fix(fdc3) - Fixing flaky DesktopAgent tests #537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 44 additions & 5 deletions src/fdc3/dotnet/DesktopAgent/src/DesktopAgent/Fdc3DesktopAgent.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading