Skip to content

Conversation

phn-ms
Copy link
Contributor

@phn-ms phn-ms commented Jul 15, 2025

  • Updated Messaging.sln and DesktopAgent.sln to include MorganStanley.ComposeUI.Messaging.MessageRouterAdapter.Tests project.
  • Enhanced logging functionality in UserChannelErrorsAndDiagnosticsTests.cs with new test methods.
  • Added tests in UserChannelTests.cs for handling null payloads and disposal behavior.
  • Created TestChannel class in TestChannel.cs for testing internal logging methods.
  • Introduced MessageRouterAdapterTests.cs to validate exception handling in ConnectAsync and InvokeAsync.
  • Defined test project in MorganStanley.ComposeUI.Messaging.MessageRouterAdapter.Tests.csproj with necessary dependencies.

@phn-ms phn-ms self-assigned this Jul 15, 2025
@phn-ms phn-ms requested a review from a team as a code owner July 15, 2025 11:10
@phn-ms phn-ms requested review from ZKRobi, fhubi, kruplm and lilla28 July 15, 2025 11:11
@codecov
Copy link

codecov bot commented Jul 15, 2025

Codecov Report

❌ Patch coverage is 40.51724% with 138 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.8%. Comparing base (dc84353) to head (d897183).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/shell/dotnet/src/Shell/Layout/LayoutManager.cs 0.0% 117 Missing ⚠️
...ng/dotnet/src/Client/Client/MessageRouterClient.cs 76.0% 3 Missing and 3 partials ⚠️
...src/Shell/Modules/WebWindowOptionsStartupAction.cs 0.0% 6 Missing ⚠️
...ll/dotnet/src/Shell/Layout/MainWindowParameters.cs 0.0% 5 Missing ⚠️
...Stanley.ComposeUI.DesktopAgent/Channels/Channel.cs 97.1% 1 Missing and 1 partial ⚠️
.../MorganStanley.ComposeUI.DesktopAgent/Fdc3Topic.cs 66.6% 1 Missing ⚠️
...uleLoader.Abstractions/ModuleInstanceExtensions.cs 0.0% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (40.5%) is below the target coverage (100.0%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #1051     +/-   ##
=======================================
+ Coverage   65.6%   65.8%   +0.1%     
=======================================
  Files        337     337             
  Lines       9979    9989     +10     
  Branches    1157    1107     -50     
=======================================
+ Hits        6547    6573     +26     
+ Misses      3164    3151     -13     
+ Partials     268     265      -3     
Files with missing lines Coverage Δ
....ComposeUI.DesktopAgent/Channels/PrivateChannel.cs 0.0% <ø> (ø)
...UI.DesktopAgent/Converters/ContextJsonConverter.cs 100.0% <100.0%> (ø)
.../MorganStanley.ComposeUI.DesktopAgent/Fdc3Topic.cs 80.0% <66.6%> (ø)
...uleLoader.Abstractions/ModuleInstanceExtensions.cs 0.0% <0.0%> (ø)
...Stanley.ComposeUI.DesktopAgent/Channels/Channel.cs 97.2% <97.1%> (+13.0%) ⬆️
...ll/dotnet/src/Shell/Layout/MainWindowParameters.cs 0.0% <0.0%> (ø)
...ng/dotnet/src/Client/Client/MessageRouterClient.cs 83.0% <76.0%> (+0.2%) ⬆️
...src/Shell/Modules/WebWindowOptionsStartupAction.cs 0.0% <0.0%> (ø)
src/shell/dotnet/src/Shell/Layout/LayoutManager.cs 0.0% <0.0%> (ø)

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

psmulovics
psmulovics previously approved these changes Jul 15, 2025
kruplm

This comment was marked as outdated.

Copy link
Contributor

@lilla28 lilla28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please do rebase and add the tests based on the changes that were applied to the repository.

@phn-ms phn-ms force-pushed the messagerouter_decoupling_part2 branch 2 times, most recently from 30372ed to 868a35a Compare August 7, 2025 09:47
@phn-ms phn-ms changed the title Refactor ComposeUI Desktop Agent project to be independent of the IMessagingService implementation /2 Unit tests for MorganStanley.ComposeUI.Messaging.MessageRouterAdapter Aug 7, 2025
@phn-ms phn-ms dismissed stale reviews from kruplm and lilla28 August 13, 2025 13:58

Changes have been implemented.

lilla28 and others added 6 commits August 14, 2025 15:23
Introduce new test projects and unit tests for MessageRouterAdapter and FDC3 DesktopAgent channels, improving coverage of exception handling, logging, and disposal logic. Refactor MessageRouterClient methods for readability, add defensive checks, and update object initialization. Enable InternalsVisibleTo for test access. No breaking changes to public APIs.
Moved <ImplicitUsings>enable</ImplicitUsings> from individual project files to Directory.Build.props for centralized management. This ensures implicit usings are enabled for all projects without redundant configuration. No other functional changes were made.
Refactored multiple files and classes to use file-scoped namespaces, reducing indentation and improving code clarity. Updated class declarations to top-level and internal where appropriate. No functional changes; these updates modernize the codebase, enhance readability, and align with current C# best practices.
The LogUnexpectedMessage method and its usages have been removed from the Channel and TestChannel classes. The associated unit test, LogUnexpectedMessage_LoggerEnabled_LogsDebugMessage, has also been deleted. This change eliminates logging for unexpected messages when closing a PrivateChannel and removes the related test coverage.
chore: Removed the <ImplicitUsings>enable</ImplicitUsings> property from MorganStanley.ComposeUI.AppDirectory.csproj, MorganStanley.ComposeUI.Messaging.MessageRouterAdapter.csproj, and MorganStanley.ComposeUI.ModuleLoader.Abstractions.csproj. Implicit using directive is defined in Directory.Build.props
@phn-ms phn-ms force-pushed the messagerouter_decoupling_part2 branch from e41f33e to 3319fba Compare August 14, 2025 13:42
The README.md file was cleared of all content, including the project overview, setup instructions, code examples, and installation guidance for the ComposeUI FDC3 Desktop Agent. No replacement content was added.
Updated the test code to await Connect() directly after disposing the channel, removing the redundant AsTask() call. This streamlines the test and reflects changes in the Connect() method's return type or usage.
@phn-ms phn-ms merged commit fc361d7 into morganstanley:main Aug 14, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants