Skip to content

Conversation

@msynk
Copy link
Member

@msynk msynk commented Nov 25, 2025

closes #11750

Summary by CodeRabbit

  • Tests
    • Added comprehensive unit test coverage for modal service functionality, including modal rendering, closing, and persistent modal behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025

Walkthrough

Introduces unit tests for BitModalService with three test methods covering modal rendering, closing, and persistent modal scenarios. Adds a test Blazor component TestModalContent to support modal content rendering in tests.

Changes

Cohort / File(s) Summary
BitModalService Tests
src/BlazorUI/Bit.BlazorUI.Tests/Components/Extras/ModalService/BitModalServiceTests.cs
New test class with SetupServices() initializer and three async test methods: renders modal in container with message validation, closes modal and verifies removal, and renders persistent modal after container initialization. Uses bunit framework with WaitForAssertion for DOM verification.
Test Modal Component
src/BlazorUI/Bit.BlazorUI.Tests/Components/Extras/ModalService/TestModalContent.cs
New Blazor component rendering a paragraph element with class "test-modal-content" displaying a parameterized Message string. Inherits from ComponentBase with manual BuildRenderTree implementation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Tests are straightforward with clear, single-purpose assertions per method
  • Minimal logic density; primarily framework-based testing code
  • Two cohesive, homogeneous files focused on one feature area
  • Areas for attention: Verify test coverage comprehensively addresses BitModalService scenarios; confirm TestModalContent aligns with actual modal content structure used in production

Poem

🐰 A modal in a container placed,
With tests now carefully traced,
Show, close, and persist so true,
The bunit rabbit brings review!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly and specifically summarizes the main change: developing unit tests for the BitModalService component, with issue reference.
Linked Issues check ✅ Passed The pull request successfully adds unit tests for BitModalService [#11750], covering modal rendering, closing, and persistent modal scenarios as required.
Out of Scope Changes check ✅ Passed All changes are directly related to the objective of creating unit tests for BitModalService; no out-of-scope modifications were introduced.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/BlazorUI/Bit.BlazorUI.Tests/Components/Extras/ModalService/BitModalServiceTests.cs (2)

20-37: Tighten content assertion to target the rendered element

Using container.Markup.Contains(message) works but is a bit loose; you can assert against the specific element rendered by TestModalContent for a more robust test:

-        container.WaitForAssertion(() =>
-        {
-            Assert.AreEqual(1, container.FindAll(".bit-mdl").Count);
-            Assert.IsTrue(container.Markup.Contains(message));
-        });
+        container.WaitForAssertion(() =>
+        {
+            Assert.AreEqual(1, container.FindAll(".bit-mdl").Count);
+
+            var content = container.Find(".test-modal-content");
+            Assert.AreEqual(message, content.TextContent);
+        });

This ties the assertion directly to the modal content component rather than any occurrence of the text in the markup.


59-72: Consider optional extra tests for overlay and persistent behaviors

Current tests cover basic render/close and the “persistent before container init” path. As a future enhancement (not blocking this PR), you might add:

  • A test that shows a non‑blocking modal and verifies that clicking the overlay closes it.
  • A test that shows a persistent modal after the container is already initialized to confirm it doesn’t get queued and still behaves as expected.

These would round out coverage of persistent and Blocking‑related behavior.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 499781a and 5c9b2ec.

📒 Files selected for processing (2)
  • src/BlazorUI/Bit.BlazorUI.Tests/Components/Extras/ModalService/BitModalServiceTests.cs (1 hunks)
  • src/BlazorUI/Bit.BlazorUI.Tests/Components/Extras/ModalService/TestModalContent.cs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/BlazorUI/Bit.BlazorUI.Tests/Components/Extras/ModalService/BitModalServiceTests.cs (4)
src/BlazorUI/Bit.BlazorUI.Extras/Components/ModalService/BitModalService.cs (1)
  • BitModalService (9-157)
src/BlazorUI/Bit.BlazorUI.Extras/Components/ModalService/BitModalContainer.razor.cs (1)
  • BitModalContainer (5-73)
src/BlazorUI/Bit.BlazorUI.Tests/Components/Extras/ModalService/TestModalContent.cs (1)
  • TestModalContent (6-17)
src/BlazorUI/Bit.BlazorUI.Extras/Components/ModalService/BitModalReference.cs (1)
  • Close (49-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build Bit.BlazorUI
🔇 Additional comments (3)
src/BlazorUI/Bit.BlazorUI.Tests/Components/Extras/ModalService/TestModalContent.cs (1)

6-16: Simple, deterministic test component looks good

The component cleanly exposes Message and renders a single predictable <p> element, which is ideal for bUnit assertions; no changes needed here.

src/BlazorUI/Bit.BlazorUI.Tests/Components/Extras/ModalService/BitModalServiceTests.cs (2)

12-18: DI setup for BitModalService in tests is appropriate

Registering BitModalService in [TestInitialize] and resolving it via the Services property integrates cleanly with BitModalContainer’s injection and keeps each test self‑contained.


39-57: Close‑path test effectively exercises BitModalReference & service wiring

The second test correctly verifies that Show renders a modal and that invoking modalRef.Close() propagates through BitModalService.Close and the container’s OnCloseModal handler, with WaitForAssertion guarding against async timing. This is a solid coverage of the close flow.

@msynk msynk merged commit 5aaca72 into bitfoundation:develop Nov 25, 2025
3 checks passed
@msynk msynk deleted the 11750-blazorui-modalservice-tests branch November 25, 2025 10:13
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.

There are no unit tests developed for the BitModalService class and its utilities

1 participant