From 24b0e00baad875b01d7c3f9d7c636629587674e6 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 6b52c648..4830680e 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 6740ca10ecf47ab0e9a91a0e748e1239cdd209e3 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 d6d30077..b9c62a39 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 47b784fc7850894901b720a4db35aa5c842b79ff 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 c1a7332a45f3e6fa5fcb2be41c79686d696076f7 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 f594b052e15dff04ebc48461186139c33f681f4a 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 8f79eb48f951d9af241fa06b92fe022289805b43 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 fca69920977ed4303ee44e4b7f5c9b5accec34b7 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 52e68010baa7b7ca45eb3af87a7a481a73c57071 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 e4e93d68841f8f1b3fdcad2bad4b9ea05c119549 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 964aceff125aaa785941d6c97747cf1e8af35e32 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 b9c62a39..8dee0817 100644 --- a/src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj +++ b/src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj @@ -44,7 +44,6 @@ -