From 30b51c0387c7c05c67a94b5fea6fbccbe5bd0f46 Mon Sep 17 00:00:00 2001 From: Mark Ridgwell <273118822+dnyw4l3n13@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:59:15 +0000 Subject: [PATCH 01/10] feat: make DiscordChannelAdapter testable and add unit tests Prompt: Work on issue #374 in funfair-tech/BuildBot. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d39e792e..780e2628 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Please ADD ALL Changes to the UNRELEASED SECTION and not a specific release - SnsMessage: Token property was never populated from the constructor argument - Added null guards for botMessageChannel and botReleaseMessageChannel parameters in BotService constructor - Pass ILogger to HealthCheckClient.ExecuteAsync to match new API in Credfeto.Docker.HealthCheck.Http.Client 0.0.72.928 +- Made DiscordChannelAdapter testable by accepting ITextChannel instead of the sealed SocketTextChannel, and added unit tests ### Changed - Dependencies - Updated NSubstitute.Analyzers.CSharp to 1.0.17 - Switched to use minimal APIs From 8a1f945b52a98616aa3ed4ae5998a0639972480a Mon Sep 17 00:00:00 2001 From: Mark Ridgwell <273118822+dnyw4l3n13@users.noreply.github.com> Date: Thu, 2 Jul 2026 16:19:04 +0000 Subject: [PATCH 02/10] feat: make DiscordChannelAdapter testable and add unit tests - Changed DiscordChannelAdapter constructor from sealed SocketTextChannel to ITextChannel interface - Changed SendMessageAsync return type from RestUserMessage to IUserMessage - Made DiscordChannelAdapter public so tests can instantiate it directly (InternalsVisibleTo is prohibited by FFS0051) - Added explicit Discord.Net package reference to BuildBot.Discord.Tests project - Added unit tests for Name, EnterTypingState, and SendMessageAsync Prompt: Work on pull request #387 in funfair-tech/BuildBot. --- .../BuildBot.Discord.Tests.csproj | 3 +- .../Services/DiscordChannelAdapterTests.cs | 73 +++++++++++++++++++ .../Services/DiscordChannelAdapter.cs | 10 +-- 3 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs diff --git a/src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj b/src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj index a05fb59c..a3b7259d 100644 --- a/src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj +++ b/src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj @@ -44,6 +44,7 @@ + @@ -70,4 +71,4 @@ - \ No newline at end of file + diff --git a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs new file mode 100644 index 00000000..1e90bdf4 --- /dev/null +++ b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs @@ -0,0 +1,73 @@ +using System; +using System.Threading.Tasks; +using BuildBot.Discord.Services; +using Discord; +using FunFair.Test.Common; +using NSubstitute; +using Xunit; + +namespace BuildBot.Discord.Tests.Services; + +public sealed class DiscordChannelAdapterTests : TestBase +{ + [Fact] + public void Name_ReturnsChannelName() + { + ITextChannel channel = GetSubstitute(); + channel.Name.Returns("test-channel"); + + DiscordChannelAdapter adapter = new(channel); + + Assert.Equal(expected: "test-channel", actual: adapter.Name); + } + + [Fact] + public void EnterTypingState_ReturnsTypingStateFromChannel() + { + ITextChannel channel = GetSubstitute(); + IDisposable typingState = GetSubstitute(); + channel.EnterTypingState(Arg.Any()).Returns(typingState); + + DiscordChannelAdapter adapter = new(channel); + + IDisposable result = adapter.EnterTypingState(); + + Assert.Same(expected: typingState, actual: result); + } + + [Fact] + public async Task SendMessageAsync_ReturnsChannelNameAndCleanContent() + { + ITextChannel channel = GetSubstitute(); + IUserMessage message = GetSubstitute(); + IMessageChannel messageChannel = GetSubstitute(); + + messageChannel.Name.Returns("sent-channel"); + message.Channel.Returns(messageChannel); + message.CleanContent.Returns("clean content"); + + channel + .SendMessageAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any() + ) + .Returns(Task.FromResult(message)); + + DiscordChannelAdapter adapter = new(channel); + + Embed embed = new EmbedBuilder().Build(); + (string sentToChannel, string messageContent) = await adapter.SendMessageAsync(embed); + + Assert.Equal(expected: "sent-channel", actual: sentToChannel); + Assert.Equal(expected: "clean content", actual: messageContent); + } +} diff --git a/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs b/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs index cf64a6cd..1686ede9 100644 --- a/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs +++ b/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs @@ -1,16 +1,14 @@ using System; using System.Threading.Tasks; using Discord; -using Discord.Rest; -using Discord.WebSocket; namespace BuildBot.Discord.Services; -internal sealed class DiscordChannelAdapter : IDiscordChannel +public sealed class DiscordChannelAdapter : IDiscordChannel { - private readonly SocketTextChannel _channel; + private readonly ITextChannel _channel; - public DiscordChannelAdapter(SocketTextChannel channel) + public DiscordChannelAdapter(ITextChannel channel) { this._channel = channel; } @@ -24,7 +22,7 @@ public IDisposable EnterTypingState() public async Task<(string SentToChannel, string MessageContent)> SendMessageAsync(Embed embed) { - RestUserMessage msg = await this._channel.SendMessageAsync(text: string.Empty, embed: embed); + IUserMessage msg = await this._channel.SendMessageAsync(text: string.Empty, embed: embed); return (msg.Channel.Name, msg.CleanContent); } From 2a0c0a2186fc24acdef90b235f4a042688778744 Mon Sep 17 00:00:00 2001 From: Mark Ridgwell <273118822+dnyw4l3n13@users.noreply.github.com> Date: Thu, 2 Jul 2026 19:24:20 +0000 Subject: [PATCH 03/10] refactor(tests): remove unnecessary IMessageChannel substitute in SendMessageAsync test ITextChannel extends IMessageChannel, so the channel substitute already satisfies the IMessageChannel type required by message.Channel. The separate messageChannel substitute was redundant. Co-Authored-By: Claude Sonnet 4.6 --- .../Services/DiscordChannelAdapterTests.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs index 1e90bdf4..72f30430 100644 --- a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs +++ b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs @@ -40,10 +40,9 @@ public async Task SendMessageAsync_ReturnsChannelNameAndCleanContent() { ITextChannel channel = GetSubstitute(); IUserMessage message = GetSubstitute(); - IMessageChannel messageChannel = GetSubstitute(); - messageChannel.Name.Returns("sent-channel"); - message.Channel.Returns(messageChannel); + channel.Name.Returns("sent-channel"); + message.Channel.Returns(channel); message.CleanContent.Returns("clean content"); channel From 8ea346b54545419827cf315d4e621ff646e206ec Mon Sep 17 00:00:00 2001 From: Mark Ridgwell <273118822+dnyw4l3n13@users.noreply.github.com> Date: Thu, 2 Jul 2026 19:28:23 +0000 Subject: [PATCH 04/10] refactor(tests): use ReturnsForAnyArgs for SendMessageAsync mock setup Replaces the 11-parameter Arg.Any<>() chain with ReturnsForAnyArgs so the mock setup is resilient to future parameter additions to the Discord.Net IMessageChannel.SendMessageAsync overload. Co-Authored-By: Claude Sonnet 4.6 --- .../Services/DiscordChannelAdapterTests.cs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs index 72f30430..bda3f4c5 100644 --- a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs +++ b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs @@ -45,21 +45,7 @@ public async Task SendMessageAsync_ReturnsChannelNameAndCleanContent() message.Channel.Returns(channel); message.CleanContent.Returns("clean content"); - channel - .SendMessageAsync( - Arg.Any(), - Arg.Any(), - Arg.Any(), - Arg.Any(), - Arg.Any(), - Arg.Any(), - Arg.Any(), - Arg.Any(), - Arg.Any(), - Arg.Any(), - Arg.Any() - ) - .Returns(Task.FromResult(message)); + channel.SendMessageAsync(text: string.Empty, embed: default).ReturnsForAnyArgs(Task.FromResult(message)); DiscordChannelAdapter adapter = new(channel); From b7a42d9a1b090358850b55810793f00ef774f098 Mon Sep 17 00:00:00 2001 From: Mark Ridgwell <273118822+dnyw4l3n13@users.noreply.github.com> Date: Thu, 2 Jul 2026 19:30:23 +0000 Subject: [PATCH 05/10] test: add Received() assertion to verify embed is forwarded in SendMessageAsync The mock setup used Arg.Any<>() for all arguments so a broken adapter passing the wrong embed or text would still pass. Adding a Received(1) call verifies the channel was called with text: string.Empty and the correct embed value. Co-Authored-By: Claude Sonnet 4.6 --- .../Services/DiscordChannelAdapterTests.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs index bda3f4c5..e2cbe7cc 100644 --- a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs +++ b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs @@ -54,5 +54,21 @@ public async Task SendMessageAsync_ReturnsChannelNameAndCleanContent() Assert.Equal(expected: "sent-channel", actual: sentToChannel); Assert.Equal(expected: "clean content", actual: messageContent); + + await channel + .Received(1) + .SendMessageAsync( + text: string.Empty, + isTTS: Arg.Any(), + embed: embed, + options: Arg.Any(), + allowedMentions: Arg.Any(), + messageReference: Arg.Any(), + components: Arg.Any(), + stickers: Arg.Any(), + embeds: Arg.Any(), + flags: Arg.Any(), + poll: Arg.Any() + ); } } From 46862a748b5202ae437d38ace997d2cdd01f9449 Mon Sep 17 00:00:00 2001 From: Mark Ridgwell <273118822+dnyw4l3n13@users.noreply.github.com> Date: Thu, 2 Jul 2026 19:32:56 +0000 Subject: [PATCH 06/10] docs(tests): clarify that CleanContent is always empty in production The adapter always sends with text: string.Empty (embed-only), so msg.CleanContent is always "" at runtime. The test stub documents this is verifying pass-through behaviour, not a realistic value. Co-Authored-By: Claude Sonnet 4.6 --- .../Services/DiscordChannelAdapterTests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs index e2cbe7cc..0146f2cd 100644 --- a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs +++ b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs @@ -43,6 +43,9 @@ public async Task SendMessageAsync_ReturnsChannelNameAndCleanContent() channel.Name.Returns("sent-channel"); message.Channel.Returns(channel); + + // In production CleanContent is always "" because the adapter sends text: string.Empty (embed-only). + // This stub verifies the adapter correctly passes through whatever CleanContent the message returns. message.CleanContent.Returns("clean content"); channel.SendMessageAsync(text: string.Empty, embed: default).ReturnsForAnyArgs(Task.FromResult(message)); From 353bbf1188c53891e0aafa2dc21940bf7bcea663 Mon Sep 17 00:00:00 2001 From: Mark Ridgwell <273118822+dnyw4l3n13@users.noreply.github.com> Date: Thu, 2 Jul 2026 20:07:36 +0000 Subject: [PATCH 07/10] fix: add null-guard on ITextChannel constructor parameter in DiscordChannelAdapter Follows project convention (see BotService) of guarding all injected constructor parameters with ?? throw new ArgumentNullException. Co-Authored-By: Claude Sonnet 4.6 --- src/BuildBot.Discord/Services/DiscordChannelAdapter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs b/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs index 1686ede9..bb23ecf0 100644 --- a/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs +++ b/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs @@ -10,7 +10,7 @@ public sealed class DiscordChannelAdapter : IDiscordChannel public DiscordChannelAdapter(ITextChannel channel) { - this._channel = channel; + this._channel = channel ?? throw new ArgumentNullException(nameof(channel)); } public string Name => this._channel.Name; From 7e70d9a5f6ecf5e5d1c3c754a2ca960c6fb25d54 Mon Sep 17 00:00:00 2001 From: Mark Ridgwell <273118822+dnyw4l3n13@users.noreply.github.com> Date: Thu, 2 Jul 2026 20:10:31 +0000 Subject: [PATCH 08/10] docs(tests): remove comment describing test intent from DiscordChannelAdapterTests The comment described what the test does rather than a non-obvious why, violating the code-quality rule. The production constraint (CleanContent is always empty for embed-only messages) is retained as the sole comment. Co-Authored-By: Claude Sonnet 4.6 --- .../Services/DiscordChannelAdapterTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs index 0146f2cd..b5b8beef 100644 --- a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs +++ b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs @@ -45,7 +45,6 @@ public async Task SendMessageAsync_ReturnsChannelNameAndCleanContent() message.Channel.Returns(channel); // In production CleanContent is always "" because the adapter sends text: string.Empty (embed-only). - // This stub verifies the adapter correctly passes through whatever CleanContent the message returns. message.CleanContent.Returns("clean content"); channel.SendMessageAsync(text: string.Empty, embed: default).ReturnsForAnyArgs(Task.FromResult(message)); From be6b0216cb3aa73c4006fb04f5b90120b8023025 Mon Sep 17 00:00:00 2001 From: Mark Ridgwell <273118822+dnyw4l3n13@users.noreply.github.com> Date: Thu, 2 Jul 2026 20:22:42 +0000 Subject: [PATCH 09/10] refactor: use this._channel.Name directly in SendMessageAsync instead of navigating msg.Channel.Name Prompt: Work on pull request #387 in funfair-tech/BuildBot. --- .../Services/DiscordChannelAdapterTests.cs | 1 - src/BuildBot.Discord/Services/DiscordChannelAdapter.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs index b5b8beef..34ed239f 100644 --- a/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs +++ b/src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs @@ -42,7 +42,6 @@ public async Task SendMessageAsync_ReturnsChannelNameAndCleanContent() IUserMessage message = GetSubstitute(); channel.Name.Returns("sent-channel"); - message.Channel.Returns(channel); // In production CleanContent is always "" because the adapter sends text: string.Empty (embed-only). message.CleanContent.Returns("clean content"); diff --git a/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs b/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs index bb23ecf0..a9e0fd99 100644 --- a/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs +++ b/src/BuildBot.Discord/Services/DiscordChannelAdapter.cs @@ -24,6 +24,6 @@ public IDisposable EnterTypingState() { IUserMessage msg = await this._channel.SendMessageAsync(text: string.Empty, embed: embed); - return (msg.Channel.Name, msg.CleanContent); + return (this._channel.Name, msg.CleanContent); } } From 6726355aff930c3f6b8fadf83280d3b1351d1109 Mon Sep 17 00:00:00 2001 From: Mark Ridgwell <273118822+dnyw4l3n13@users.noreply.github.com> Date: Thu, 2 Jul 2026 20:26:10 +0000 Subject: [PATCH 10/10] refactor(tests): remove redundant Discord.Net PackageReference from test project Discord.Net is available transitively via the ProjectReference to BuildBot.Discord.csproj, so declaring it explicitly creates version-skew risk. Prompt: Work on pull request #387 in funfair-tech/BuildBot. --- src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj b/src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj index a3b7259d..b687ed47 100644 --- a/src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj +++ b/src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj @@ -44,7 +44,6 @@ -