Skip to content

feat: make DiscordChannelAdapter testable and add unit tests#387

Merged
dnyw4l3n13 merged 10 commits into
mainfrom
feature/374-discord-channel-adapter-testable
Jul 8, 2026
Merged

feat: make DiscordChannelAdapter testable and add unit tests#387
dnyw4l3n13 merged 10 commits into
mainfrom
feature/374-discord-channel-adapter-testable

Conversation

@dnyw4l3n13

@dnyw4l3n13 dnyw4l3n13 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

Makes DiscordChannelAdapter testable by:

  • Changing constructor parameter from the sealed SocketTextChannel to the ITextChannel interface
  • Widening RestUserMessage return type to IUserMessage in SendMessageAsync
  • Adding [assembly: InternalsVisibleTo("BuildBot.Discord.Tests")] so tests can instantiate the internal class
  • Adding explicit Discord.Net package reference to the test project
  • Adding unit tests for Name, EnterTypingState, and SendMessageAsync in DiscordChannelAdapter

Test plan

  • Name_ReturnsChannelName — verifies Name delegates to ITextChannel.Name
  • EnterTypingState_ReturnsTypingStateFromChannel — verifies EnterTypingState() returns the IDisposable from the channel
  • SendMessageAsync_ReturnsChannelNameAndCleanContent — verifies the tuple mapping from IUserMessage

Closes #374

@dnyw4l3n13 dnyw4l3n13 self-assigned this Jul 2, 2026
@dnyw4l3n13 dnyw4l3n13 added AI-Work Work for an AI Agent Tech Debt Technical debt buildbot-discord Changes in BuildBot.Discord project labels Jul 2, 2026
dnyw4l3n13 added a commit that referenced this pull request Jul 2, 2026
@dnyw4l3n13
feat: make DiscordChannelAdapter testable and add unit tests
b44b951 
- 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.
@dnyw4l3n13

dnyw4l3n13 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Implemented missing code changes in commit b44b951: changed DiscordChannelAdapter constructor from SocketTextChannel to ITextChannel, widened return type to IUserMessage, made class public (InternalsVisibleTo is prohibited by FFS0051), added explicit Discord.Net reference to test project, and added unit tests for Name, EnterTypingState, and SendMessageAsync. CI running; next cycle will continue.

dnyw4l3n13
dnyw4l3n13 commented Jul 2, 2026
View reviewed changes
Comment thread src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs
dnyw4l3n13
dnyw4l3n13 commented Jul 2, 2026
View reviewed changes
Comment thread src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs
dnyw4l3n13
dnyw4l3n13 commented Jul 2, 2026
View reviewed changes
Comment thread src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs Outdated
dnyw4l3n13
dnyw4l3n13 commented Jul 2, 2026
View reviewed changes
Comment thread src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs Outdated
@dnyw4l3n13

dnyw4l3n13 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Code review round: fixed 4 findings — removed unnecessary IMessageChannel substitute, switched to ReturnsForAnyArgs for mock resilience, added Received(1) assertion to verify embed forwarding, and clarified CleanContent pass-through behaviour in test comment. Next cycle will continue with security review if CI is green.

dnyw4l3n13
dnyw4l3n13 commented Jul 2, 2026
View reviewed changes
Comment thread src/BuildBot.Discord/Services/DiscordChannelAdapter.cs
dnyw4l3n13
dnyw4l3n13 commented Jul 2, 2026
View reviewed changes
Comment thread src/BuildBot.Discord.Tests/BuildBot.Discord.Tests.csproj Outdated
dnyw4l3n13
dnyw4l3n13 commented Jul 2, 2026
View reviewed changes
Comment thread src/BuildBot.Discord/Services/DiscordChannelAdapter.cs
dnyw4l3n13
dnyw4l3n13 commented Jul 2, 2026
View reviewed changes
Comment thread src/BuildBot.Discord/Services/DiscordChannelAdapter.cs Outdated
dnyw4l3n13
dnyw4l3n13 commented Jul 2, 2026
View reviewed changes
Comment thread src/BuildBot.Discord.Tests/Services/DiscordChannelAdapterTests.cs Outdated
@dnyw4l3n13

dnyw4l3n13 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Code review complete — 2 confirmed findings and 1 plausible finding posted as inline comments: (1) class should be internal + InternalsVisibleTo rather than public, (2) missing null-guard on constructor parameter, (3) test comment line 48 describes what the test does rather than a non-obvious why. Fixing now.

@dnyw4l3n13

dnyw4l3n13 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Code review round: fixed 2 findings — added null-guard on DiscordChannelAdapter constructor parameter (e6bbfa6), removed redundant test comment describing test intent (ff19af1). Note: the InternalsVisibleTo finding was withdrawn as FunFair.CodeAnalysis rule FFS0051 prohibits that pattern; making the class public is the correct approach for this project.

dnyw4l3n13 added a commit that referenced this pull request Jul 2, 2026
@dnyw4l3n13
refactor: use this._channel.Name directly in SendMessageAsync instead…
a37cac5 
… of navigating msg.Channel.Name

Prompt: Work on pull request #387 in funfair-tech/BuildBot.
dnyw4l3n13 added a commit that referenced this pull request Jul 2, 2026
@dnyw4l3n13
refactor(tests): remove redundant Discord.Net PackageReference from t…
488d96c 
…est 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.
@dnyw4l3n13

dnyw4l3n13 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Code review clean — fixed 2 remaining inline findings (use this._channel.Name directly, remove redundant Discord.Net PackageReference) and advancing to security review.

@dnyw4l3n13

dnyw4l3n13 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Security review clean — no findings. Advancing to Human Review.

@dnyw4l3n13 dnyw4l3n13 marked this pull request as ready for review July 2, 2026 20:42
@dnyw4l3n13 dnyw4l3n13 requested a review from a team as a code owner July 2, 2026 20:42
@dnyw4l3n13 dnyw4l3n13 enabled auto-merge July 2, 2026 20:42
@dnyw4l3n13

dnyw4l3n13 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

PR is ready for merge — marked ready for review, auto-merge enabled. Code review and security review both clean.

@credfeto

credfeto commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Super-linter summary

Language Validation result

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

dnyw4l3n13 added a commit that referenced this pull request Jul 2, 2026
@dnyw4l3n13
feat: make DiscordChannelAdapter testable and add unit tests
605c1bb 
- 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.
dnyw4l3n13 added a commit that referenced this pull request Jul 2, 2026
@dnyw4l3n13
refactor: use this._channel.Name directly in SendMessageAsync instead…
b3fadd7 
… of navigating msg.Channel.Name

Prompt: Work on pull request #387 in funfair-tech/BuildBot.
dnyw4l3n13 added a commit that referenced this pull request Jul 2, 2026
@dnyw4l3n13
refactor(tests): remove redundant Discord.Net PackageReference from t…
7589940 
…est 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.
dnyw4l3n13 added a commit that referenced this pull request Jul 4, 2026
@dnyw4l3n13
feat: make DiscordChannelAdapter testable and add unit tests
3fdc84a 
- 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.
@dnyw4l3n13 dnyw4l3n13 force-pushed the feature/374-discord-channel-adapter-testable branch from 690d2ae to c0ee30c Compare July 4, 2026 10:56
dnyw4l3n13 added a commit that referenced this pull request Jul 4, 2026
@dnyw4l3n13
refactor: use this._channel.Name directly in SendMessageAsync instead…
5abdfff 
… of navigating msg.Channel.Name

Prompt: Work on pull request #387 in funfair-tech/BuildBot.
dnyw4l3n13 added a commit that referenced this pull request Jul 4, 2026
@dnyw4l3n13
refactor(tests): remove redundant Discord.Net PackageReference from t…
c0ee30c 
…est 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.
dnyw4l3n13 added a commit that referenced this pull request Jul 4, 2026
@dnyw4l3n13
feat: make DiscordChannelAdapter testable and add unit tests
c27cd79 
- 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.
dnyw4l3n13 added a commit that referenced this pull request Jul 4, 2026
@dnyw4l3n13
refactor: use this._channel.Name directly in SendMessageAsync instead…
75092f9 
… of navigating msg.Channel.Name

Prompt: Work on pull request #387 in funfair-tech/BuildBot.
dnyw4l3n13 added a commit that referenced this pull request Jul 4, 2026
@dnyw4l3n13
refactor(tests): remove redundant Discord.Net PackageReference from t…
22defff 
…est 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.
@dnyw4l3n13 dnyw4l3n13 force-pushed the feature/374-discord-channel-adapter-testable branch from c0ee30c to 22defff Compare July 4, 2026 15:16
dnyw4l3n13 added a commit that referenced this pull request Jul 4, 2026
@dnyw4l3n13
feat: make DiscordChannelAdapter testable and add unit tests
8a1f945 
- 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.
dnyw4l3n13 added a commit that referenced this pull request Jul 4, 2026
@dnyw4l3n13
refactor: use this._channel.Name directly in SendMessageAsync instead…
be6b021 
… of navigating msg.Channel.Name

Prompt: Work on pull request #387 in funfair-tech/BuildBot.
dnyw4l3n13 added a commit that referenced this pull request Jul 4, 2026
@dnyw4l3n13
refactor(tests): remove redundant Discord.Net PackageReference from t…
6726355 
…est 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.
@dnyw4l3n13 dnyw4l3n13 force-pushed the feature/374-discord-channel-adapter-testable branch from 22defff to 6726355 Compare July 4, 2026 15:35
@dnyw4l3n13 dnyw4l3n13 added the Blocked Needs human input before work can continue label Jul 5, 2026
@dnyw4l3n13

dnyw4l3n13 commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

This PR has been worked 30 times by the automation without reaching a mergeable state. Blocking for a human to investigate.

@credfeto credfeto removed the Blocked Needs human input before work can continue label Jul 8, 2026
dnyw4l3n13 and others added 10 commits July 8, 2026 10:50
@dnyw4l3n13 @credfeto
feat: make DiscordChannelAdapter testable and add unit tests
24b0e00 
Prompt: Work on issue #374 in funfair-tech/BuildBot.
@dnyw4l3n13 @credfeto
feat: make DiscordChannelAdapter testable and add unit tests
6740ca1 
- 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.
@dnyw4l3n13 @claude @credfeto
refactor(tests): remove unnecessary IMessageChannel substitute in Sen…
47b784f 
…dMessageAsync 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 <noreply@anthropic.com>
@dnyw4l3n13 @claude @credfeto
refactor(tests): use ReturnsForAnyArgs for SendMessageAsync mock setup
c1a7332 
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 <noreply@anthropic.com>
@dnyw4l3n13 @claude @credfeto
test: add Received() assertion to verify embed is forwarded in SendMe…
f594b05 
…ssageAsync

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 <noreply@anthropic.com>
@dnyw4l3n13 @claude @credfeto
docs(tests): clarify that CleanContent is always empty in production
8f79eb4 
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 <noreply@anthropic.com>
@dnyw4l3n13 @claude @credfeto
fix: add null-guard on ITextChannel constructor parameter in DiscordC…
fca6992 
…hannelAdapter

Follows project convention (see BotService) of guarding all injected
constructor parameters with ?? throw new ArgumentNullException.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@dnyw4l3n13 @claude @credfeto
docs(tests): remove comment describing test intent from DiscordChanne…
52e6801 
…lAdapterTests

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 <noreply@anthropic.com>
@dnyw4l3n13 @credfeto
refactor: use this._channel.Name directly in SendMessageAsync instead…
e4e93d6 
… of navigating msg.Channel.Name

Prompt: Work on pull request #387 in funfair-tech/BuildBot.
@dnyw4l3n13 @credfeto
refactor(tests): remove redundant Discord.Net PackageReference from t…
964acef 
…est 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.
@credfeto credfeto force-pushed the feature/374-discord-channel-adapter-testable branch from 6726355 to 964acef Compare July 8, 2026 09:50
@dnyw4l3n13 dnyw4l3n13 merged commit 3118acf into main Jul 8, 2026
20 checks passed
@dnyw4l3n13 dnyw4l3n13 deleted the feature/374-discord-channel-adapter-testable branch July 8, 2026 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@credfeto credfeto credfeto approved these changes

Assignees

@dnyw4l3n13 dnyw4l3n13

Labels

AI-Work Work for an AI Agent buildbot-discord Changes in BuildBot.Discord project Tech Debt Technical debt

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Coverage: BuildBot.Discord — DiscordChannelAdapter needs integration-test coverage

2 participants

@dnyw4l3n13 @credfeto