test(discord): Add null-guard branch coverage for BotService constructor

[github-actions]

Summary

• Adds a reflection-based test to cover the bot == null guard in BotService constructor, bringing BotService to 100% line and branch coverage.
• The NX0002 analyser rule prevents using null! directly, so the constructor is invoked via ConstructorInfo.Invoke with a null argument instead.

Coverage status after this PR

Assembly	Line	Branch
BuildBot.ServiceModel	100%	100%
BuildBot.Discord	~93% line (infrastructure gaps tracked in #374)	see #374
BuildBot.Health	100%	100%
BuildBot.Json	100%	100%
BuildBot.Watchtower	100%	100%
BuildBot.CloudFormation	100%	100%
BuildBot.GitHub	100%	100%
BuildBot	100%	100%

Remaining gaps in BuildBot.Discord (DiscordChannelAdapter and the live-infrastructure methods of DiscordSocketClientAdapter) are infrastructure-dependent and tracked in issue #374.

Closes #356

Test plan

• All 233 tests pass
• BotService reaches 100% line and branch coverage
• dotnet buildcheck passes with no errors

[dnyw4l3n13]

[Code Review] The test asserts only IsType<ArgumentNullException> without checking ParamName. If the guard in BotService is accidentally changed to throw new ArgumentNullException(nameof(botMessageChannel)) (wrong parameter name), the test still passes and the regression goes undetected.

Suggestion: also assert the parameter name:

ArgumentNullException ane = Assert.IsType<ArgumentNullException>(ex.InnerException);
Assert.Equal("bot", ane.ParamName);

[dnyw4l3n13]

Fixed in 0e24809 — asserted ParamName == "bot" on the caught ArgumentNullException so a mis-targeted guard would be detected.

[dnyw4l3n13]

[Code Review] Assert.Single(typeof(BotService).GetConstructors()) is a guard against a second constructor being added, but if one is ever added the failure message will be a count-related xunit error with no mention of null guards — making the root cause non-obvious to the next developer.

Consider selecting the specific constructor by its parameter types instead, which both locates the right overload and provides a clearer failure if the signature changes:

ConstructorInfo? ctor = typeof(BotService).GetConstructor([
    typeof(IDiscordBot),
    typeof(IMessageChannel<BotMessage>),
    typeof(IMessageChannel<BotReleaseMessage>)
]);
Assert.NotNull(ctor);

[dnyw4l3n13]

Fixed in 0e24809 — replaced Assert.Single(GetConstructors()) with GetConstructor([...]) targeting the constructor by explicit parameter types, giving a clearer failure if the signature changes.

[dnyw4l3n13]

[Code Review] messageChannel and releaseChannel are created but never reached: the BotService constructor throws ArgumentNullException on the first line (bot ?? throw ...) before it touches either channel parameter. Passing null for those positions (or any placeholder) would produce the same result and make it clearer that only bot is the subject under test.

[dnyw4l3n13]

Fixed in 0e24809 — removed the unnecessary messageChannel and releaseChannel allocations; all three invoke arguments are now null since the constructor throws before reaching the channel parameters.

[dnyw4l3n13]

[Code Review — Pre-existing gap] BotService.cs lines 27-28 assign botMessageChannel and botReleaseMessageChannel without null guards, but both are immediately dereferenced inside the Rx chain (lines 31 and 41). If either is passed as null, a NullReferenceException fires deep in Rx internals with no hint that it was a missing null guard. This is pre-existing and outside this PR's diff, but it is the subject matter this PR is extending (null-guard coverage), so flagging it here for tracking. Consider adding ?? throw new ArgumentNullException(nameof(...)) guards for the two channel parameters in BotService.cs.

[dnyw4l3n13]

Fixed in ee1e59b — added ?? throw new ArgumentNullException(nameof(...)) guards for both botMessageChannel and botReleaseMessageChannel in BotService.cs, and added corresponding constructor tests (Constructor_ThrowsArgumentNullException_WhenBotMessageChannelIsNull and Constructor_ThrowsArgumentNullException_WhenBotReleaseMessageChannelIsNull) to cover both new guards.

[dnyw4l3n13]

[Code Review] The three Constructor_ThrowsArgumentNullException_When* methods are structurally identical — each performs the same GetConstructor lookup and the same assert pattern, varying only in which argument is null and the expected ParamName. Project rules require: 'Prefer parameterised tests over duplicated test methods — each behavioural variant is a data point, not a separate method. Use the idiomatic mechanism for the framework (xUnit [Theory]/[InlineData]).' A single [Theory] with [MemberData] covers all three cases, and also eliminates the GetConstructor call duplicated verbatim three times.

[dnyw4l3n13]

Fixed in bf6ec38 — collapsed the three identical [Fact] methods into a single [Theory] with [MemberData], using an int index to select which parameter is null and setting it after pre-populating all args with non-null substitutes. The GetConstructor call now appears once.

[credfeto]

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]

All work complete. Summary of changes on this PR:

• 0e24809 — Strengthened the existing bot null-guard test: use GetConstructor with explicit parameter types, pass [null, null, null] to avoid unnecessary allocations, assert ParamName == "bot"
• ee1e59b — Added null guards for botMessageChannel and botReleaseMessageChannel in BotService.cs, with corresponding tests
• bf6ec38 — Refactored three identical [Fact] null-guard tests into a single [Theory] with [MemberData] per project rules

Code review and security review both passed clean. Auto-merge enabled.

[dnyw4l3n13]

Rebased the branch onto origin/main to pick up the Meziantou.Analyzer 3.0.115 dependency update (commit cc5c24cb). Force-pushed successfully; CI is now running on the rebased tip. Auto-merge remains enabled — will merge automatically once checks pass.

[dnyw4l3n13]

All CI checks pass and auto-merge is enabled. The PR is currently blocked on REVIEW_REQUIRED — the previous approval from @credfeto was dismissed when the branch was rebased and force-pushed (2026-06-27T17:43). No further code changes are needed; the PR is ready and waiting for a human approval to satisfy the branch-protection rule before auto-merge can complete.