Revert Sparkle manual update dialog flow from #1908#2090
Merged
austinywang merged 2 commits intomainfrom Mar 25, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR removes the Sparkle dialog presentation infrastructure and presentation-specific routing from the update flow, consolidating all update checking into a unified code path. The Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
austinywang
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/sparkle_generate_appcast.sh">
<violation number="1" location="scripts/sparkle_generate_appcast.sh:96">
P2: Do not swallow changelog extraction errors with `|| true`; it hides real failures and silently drops embedded release notes.</violation>
</file>
<file name="Sources/Update/UpdatePill.swift">
<violation number="1" location="Sources/Update/UpdatePill.swift:32">
P2: Sidebar update pill click now uses the dialog route instead of the inline/custom update flow, creating inconsistent in-app update behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| fi | ||
|
|
||
| if [[ -z "$APPCAST_CHANGELOG_TEXT" && -f "$CHANGELOG_PATH" ]]; then | ||
| APPCAST_CHANGELOG_TEXT="$(python3 scripts/appcast_changelog.py --tag "$TAG" --changelog "$CHANGELOG_PATH" || true)" |
There was a problem hiding this comment.
P2: Do not swallow changelog extraction errors with || true; it hides real failures and silently drops embedded release notes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/sparkle_generate_appcast.sh, line 96:
<comment>Do not swallow changelog extraction errors with `|| true`; it hides real failures and silently drops embedded release notes.</comment>
<file context>
@@ -90,6 +92,10 @@ if [[ ! -f "$generated_appcast_path" ]]; then
fi
+if [[ -z "$APPCAST_CHANGELOG_TEXT" && -f "$CHANGELOG_PATH" ]]; then
+ APPCAST_CHANGELOG_TEXT="$(python3 scripts/appcast_changelog.py --tag "$TAG" --changelog "$CHANGELOG_PATH" || true)"
+fi
+
</file context>
| if model.showsDetectedBackgroundUpdate { | ||
| showPopover = false | ||
| AppDelegate.shared?.checkForUpdates(nil) | ||
| AppDelegate.shared?.checkForUpdatesInDialog(nil) |
There was a problem hiding this comment.
P2: Sidebar update pill click now uses the dialog route instead of the inline/custom update flow, creating inconsistent in-app update behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Update/UpdatePill.swift, line 32:
<comment>Sidebar update pill click now uses the dialog route instead of the inline/custom update flow, creating inconsistent in-app update behavior.</comment>
<file context>
@@ -29,7 +29,7 @@ struct UpdatePill: View {
if model.showsDetectedBackgroundUpdate {
showPopover = false
- AppDelegate.shared?.checkForUpdates(nil)
+ AppDelegate.shared?.checkForUpdatesInDialog(nil)
return
}
</file context>
Suggested change
| AppDelegate.shared?.checkForUpdatesInDialog(nil) | |
| AppDelegate.shared?.checkForUpdates(nil) |
Contributor
Greptile SummaryThis PR restores the two-path update UX: the sidebar/in-app "Check for Updates" action now uses the inline Key changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant SidebarHelpMenu
participant AppMenu as App Menu / MenuBarExtra
participant BackgroundPill as UpdatePill (background update)
participant AppDelegate
participant UpdateController
participant Sparkle
Note over User,Sparkle: Inline pill flow (sidebar "Check for Updates")
User->>SidebarHelpMenu: Click "Check for Updates"
SidebarHelpMenu->>AppDelegate: checkForUpdates(nil)
AppDelegate->>UpdateController: checkForUpdates()
UpdateController->>Sparkle: requestCheckForUpdates(.custom)
Sparkle-->>AppDelegate: didFindUpdate
AppDelegate-->>User: Shows inline UpdatePill in sidebar
Note over User,Sparkle: Sparkle dialog flow (app menu / menu bar extra)
User->>AppMenu: Click "Check for Updates…"
AppMenu->>AppDelegate: checkForUpdatesInDialog(nil)
AppDelegate->>UpdateController: checkForUpdatesInDialog()
UpdateController->>Sparkle: requestCheckForUpdates(.dialog)
Sparkle-->>User: Shows Sparkle dialog (with embedded changelog)
Note over User,Sparkle: Background-detected update pill click
Sparkle->>AppDelegate: didFindUpdate (background probe)
AppDelegate-->>User: Shows UpdatePill (showsDetectedBackgroundUpdate=true)
User->>BackgroundPill: Click pill
BackgroundPill->>AppDelegate: checkForUpdatesInDialog(nil)
AppDelegate->>UpdateController: checkForUpdatesInDialog()
UpdateController->>Sparkle: requestCheckForUpdates(.dialog)
Sparkle-->>User: Shows Sparkle dialog (with embedded changelog)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
cmuxUITests/SidebarHelpMenuUITests.swift (2)
92-113: This should exercise the real menu route, not only the debug hook.The env flag proves
checkForUpdatesInDialogcan render the Sparkle dialog, but it doesn't verify that the actual menu item's action still points there. If the menu wiring regresses back to the inline path, this test stays green.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxUITests/SidebarHelpMenuUITests.swift` around lines 92 - 113, The test currently uses the debug env flag CMUX_UI_TEST_TRIGGER_UPDATE_CHECK_DIALOG which only exercises the debug hook; update testDialogTriggeredCheckForUpdatesShowsSparkleDialogWithChangelog to trigger the real menu action instead: remove or stop relying on CMUX_UI_TEST_TRIGGER_UPDATE_CHECK_DIALOG, keep the feed-related env vars (CMUX_UI_TEST_FEED_URL, CMUX_UI_TEST_FEED_MODE, CMUX_UI_TEST_UPDATE_VERSION, CMUX_UI_TEST_RELEASE_NOTES_SUMMARY), launch the app, then open the app menu and tap the actual "Check for Updates…" menu item (use XCUIApplication().menuBars.menuItems["Check for Updates…"] or the app's accessibility identifier) before asserting the Sparkle dialog appears with waitForWindowCount and checking installButton/"Remind Me Later"/"Skip This Version"/"99.0.0"/summary existence.
59-90: Please assert the inline popover contents too.Both sidebar-path tests stop after the pill appears. That still passes if the pill renders but the anchored popover never opens, or if it opens without the release-notes summary / install action that issue
#2028calls for. I'd extend these to click the pill and assert the inline popover content, not just the absence of Sparkle's modal buttons.Also applies to: 115-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxUITests/SidebarHelpMenuUITests.swift` around lines 59 - 90, The test stops after the update pill appears but must also verify the anchored inline popover and its contents; update testHelpMenuCheckForUpdatesShowsInlineUpdatePillOnFirstAttempt (and the other sidebar-path test) to click the pill (app.buttons["Update Available: 99.0.0"]), wait for the anchored popover to appear and be hittable, and then assert the presence of the release-notes summary element and the inline Install action (and optionally Remind Me Later / Skip This Version controls) inside that popover to ensure the inline UI opened correctly rather than only the pill rendering.scripts/appcast_changelog.py (1)
17-18: Please lock thev/bare-tag contract with a regression test.
sparkle_generate_appcast.shpasses$TAGthrough as-is, so these two functions are what keepv0.62.2and0.62.2equivalent. A small CLI test for both spellings would make that release-path behavior much harder to regress.Also applies to: 32-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/appcast_changelog.py` around lines 17 - 18, Add a regression test that ensures tags with and without a leading "v" produce identical results: write a unit test for normalize_tag(tag: str) that asserts normalize_tag("v0.62.2") == normalize_tag("0.62.2"), and add a small CLI/integration test that invokes the appcast generator script twice (once with TAG="v0.62.2" and once with TAG="0.62.2") and asserts the generated appcast/changelog outputs are byte-for-byte equal; place tests alongside existing test suite and target the appcast generation entrypoint that consumes TAG so the v/bare-tag contract (functions including normalize_tag and the appcast generation entrypoint) cannot regress.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/sparkle_generate_appcast.sh`:
- Around line 95-97: The command substitution that populates
APPCAST_CHANGELOG_TEXT is swallowing real errors via "|| true"; remove the "||
true" so failures in scripts/appcast_changelog.py propagate and cause the script
to fail instead of silently publishing without notes. Update the block that runs
scripts/appcast_changelog.py (the line setting APPCAST_CHANGELOG_TEXT with TAG
and CHANGELOG_PATH) to omit "|| true", and optionally add an explicit
empty-string check for APPCAST_CHANGELOG_TEXT after the call if you need
different handling for legitimately-missing changelog entries versus execution
errors.
In `@Sources/AppDelegate.swift`:
- Around line 5872-5875: The ContentView and UpdateDriver callers still call
checkForUpdates(nil) and bypass the dialog helper; update those call sites to
use the dialog entry path: clear any override by setting
updateViewModel.overrideState = nil (or ensure the view model state is cleared)
and invoke AppDelegate's checkForUpdatesInDialog (or call
updateController.checkForUpdatesInDialog()) instead of checkForUpdates(nil) so
user-triggered and retry-from-error flows consistently present Sparkle's dialog;
locate and replace the calls to checkForUpdates(nil) in the methods referenced
(Sources/ContentView.swift and Sources/Update/UpdateDriver.swift) to call the
dialog helper or ensure overrideState is nil before invoking
checkForUpdatesInDialog.
- Around line 2356-2360: The test hook fires checkForUpdatesInDialog(nil) too
early; change the DispatchQueue.main.asyncAfter block to wait until the app has
a main/visible window before calling checkForUpdatesInDialog(nil). Concretely,
after detecting env["CMUX_UI_TEST_TRIGGER_UPDATE_CHECK_DIALOG"] == "1" (and
after UpdateLogStore.shared.append), replace the single delayed call with logic
that either observes NSWindow/NSApplication window notifications (e.g.,
NSWindow.didBecomeMainNotification or NSApplication.shared.addObserver for
window activation) or polls on the main queue for
NSApplication.shared.mainWindow/visible window, and only invoke
self?.checkForUpdatesInDialog(nil) once a main/visible window exists; keep using
the same DispatchQueue/main context and preserve the 0.25s fallback timing if
desired.
In `@Sources/Update/UpdatePill.swift`:
- Around line 30-33: The current branch handling
model.showsDetectedBackgroundUpdate calls
AppDelegate.shared?.checkForUpdatesInDialog(nil), forcing Sparkle's modal dialog
instead of the intended inline/sidebar pill flow; replace that call with the
inline update presentation path (e.g., call the app-level method that triggers
the sidebar/inline update UI such as a hypothetical
AppDelegate.shared?.checkForUpdatesInline(...) or presentUpdatePillInline(...)
), or if no such helper exists, add one that triggers the existing
inline/sidebar update UI and call it here instead of
checkForUpdatesInDialog(nil), keeping showPopover = false and returning as
before; update references to model.showsDetectedBackgroundUpdate, showPopover,
and the AppDelegate call accordingly.
In `@Sources/Update/UpdateTestURLProtocol.swift`:
- Around line 78-80: The releaseNotesSummary string may contain the CDATA
terminator "]]>", which will break XML when interpolated; before using
releaseNotesSummary in XML (and similarly for the values used at the locations
referenced around lines 93-94), replace every occurrence of "]]>" with
"]]]]><![CDATA[>" (or otherwise split the terminator) so the CDATA block stays
valid—update the code that prepares releaseNotesSummary (and the other
release-notes variable(s) used in the XML interpolation) to perform this
replacement prior to trimming/embedding.
---
Nitpick comments:
In `@cmuxUITests/SidebarHelpMenuUITests.swift`:
- Around line 92-113: The test currently uses the debug env flag
CMUX_UI_TEST_TRIGGER_UPDATE_CHECK_DIALOG which only exercises the debug hook;
update testDialogTriggeredCheckForUpdatesShowsSparkleDialogWithChangelog to
trigger the real menu action instead: remove or stop relying on
CMUX_UI_TEST_TRIGGER_UPDATE_CHECK_DIALOG, keep the feed-related env vars
(CMUX_UI_TEST_FEED_URL, CMUX_UI_TEST_FEED_MODE, CMUX_UI_TEST_UPDATE_VERSION,
CMUX_UI_TEST_RELEASE_NOTES_SUMMARY), launch the app, then open the app menu and
tap the actual "Check for Updates…" menu item (use
XCUIApplication().menuBars.menuItems["Check for Updates…"] or the app's
accessibility identifier) before asserting the Sparkle dialog appears with
waitForWindowCount and checking installButton/"Remind Me Later"/"Skip This
Version"/"99.0.0"/summary existence.
- Around line 59-90: The test stops after the update pill appears but must also
verify the anchored inline popover and its contents; update
testHelpMenuCheckForUpdatesShowsInlineUpdatePillOnFirstAttempt (and the other
sidebar-path test) to click the pill (app.buttons["Update Available: 99.0.0"]),
wait for the anchored popover to appear and be hittable, and then assert the
presence of the release-notes summary element and the inline Install action (and
optionally Remind Me Later / Skip This Version controls) inside that popover to
ensure the inline UI opened correctly rather than only the pill rendering.
In `@scripts/appcast_changelog.py`:
- Around line 17-18: Add a regression test that ensures tags with and without a
leading "v" produce identical results: write a unit test for normalize_tag(tag:
str) that asserts normalize_tag("v0.62.2") == normalize_tag("0.62.2"), and add a
small CLI/integration test that invokes the appcast generator script twice (once
with TAG="v0.62.2" and once with TAG="0.62.2") and asserts the generated
appcast/changelog outputs are byte-for-byte equal; place tests alongside
existing test suite and target the appcast generation entrypoint that consumes
TAG so the v/bare-tag contract (functions including normalize_tag and the
appcast generation entrypoint) cannot regress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f642b2a5-213d-4abc-acb7-df311271e711
📒 Files selected for processing (10)
.github/workflows/ci.ymlSources/AppDelegate.swiftSources/Update/UpdateController.swiftSources/Update/UpdatePill.swiftSources/Update/UpdateTestURLProtocol.swiftSources/cmuxApp.swiftcmuxUITests/SidebarHelpMenuUITests.swiftscripts/appcast_changelog.pyscripts/sparkle_generate_appcast.shtests/test_appcast_changelog_extraction.sh
Comment on lines
+95
to
+97
| if [[ -z "$APPCAST_CHANGELOG_TEXT" && -f "$CHANGELOG_PATH" ]]; then | ||
| APPCAST_CHANGELOG_TEXT="$(python3 scripts/appcast_changelog.py --tag "$TAG" --changelog "$CHANGELOG_PATH" || true)" | ||
| fi |
There was a problem hiding this comment.
Don't swallow extractor failures with || true.
scripts/appcast_changelog.py already exits 0 when the changelog is missing or the tag has no section, so || true only hides real execution/read failures. In that case this script silently publishes an appcast without embedded notes and later reports it as "no matching changelog entry".
Suggested fix
-if [[ -z "$APPCAST_CHANGELOG_TEXT" && -f "$CHANGELOG_PATH" ]]; then
- APPCAST_CHANGELOG_TEXT="$(python3 scripts/appcast_changelog.py --tag "$TAG" --changelog "$CHANGELOG_PATH" || true)"
-fi
+if [[ -z "$APPCAST_CHANGELOG_TEXT" && -f "$CHANGELOG_PATH" ]]; then
+ if ! APPCAST_CHANGELOG_TEXT="$(python3 scripts/appcast_changelog.py --tag "$TAG" --changelog "$CHANGELOG_PATH")"; then
+ echo "Failed to extract changelog entry for $TAG from $CHANGELOG_PATH" >&2
+ exit 1
+ fi
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if [[ -z "$APPCAST_CHANGELOG_TEXT" && -f "$CHANGELOG_PATH" ]]; then | |
| APPCAST_CHANGELOG_TEXT="$(python3 scripts/appcast_changelog.py --tag "$TAG" --changelog "$CHANGELOG_PATH" || true)" | |
| fi | |
| if [[ -z "$APPCAST_CHANGELOG_TEXT" && -f "$CHANGELOG_PATH" ]]; then | |
| if ! APPCAST_CHANGELOG_TEXT="$(python3 scripts/appcast_changelog.py --tag "$TAG" --changelog "$CHANGELOG_PATH")"; then | |
| echo "Failed to extract changelog entry for $TAG from $CHANGELOG_PATH" >&2 | |
| exit 1 | |
| fi | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/sparkle_generate_appcast.sh` around lines 95 - 97, The command
substitution that populates APPCAST_CHANGELOG_TEXT is swallowing real errors via
"|| true"; remove the "|| true" so failures in scripts/appcast_changelog.py
propagate and cause the script to fail instead of silently publishing without
notes. Update the block that runs scripts/appcast_changelog.py (the line setting
APPCAST_CHANGELOG_TEXT with TAG and CHANGELOG_PATH) to omit "|| true", and
optionally add an explicit empty-string check for APPCAST_CHANGELOG_TEXT after
the call if you need different handling for legitimately-missing changelog
entries versus execution errors.
Comment on lines
+2356
to
+2360
| if env["CMUX_UI_TEST_TRIGGER_UPDATE_CHECK_DIALOG"] == "1" { | ||
| UpdateLogStore.shared.append("ui test trigger update dialog detected") | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { [weak self] in | ||
| self?.checkForUpdatesInDialog(nil) | ||
| } |
There was a problem hiding this comment.
Wait for a main window before firing the dialog test hook.
Line 2358 queues the dialog at the same 0.25s deadline as the later XCTest fallback that force-creates and activates a window, but this block is enqueued first. On the slow-WindowGroup path called out below, checkForUpdatesInDialog(nil) can run before any window exists, which makes the new Sparkle-dialog UI test hook flaky.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 2356 - 2360, The test hook fires
checkForUpdatesInDialog(nil) too early; change the DispatchQueue.main.asyncAfter
block to wait until the app has a main/visible window before calling
checkForUpdatesInDialog(nil). Concretely, after detecting
env["CMUX_UI_TEST_TRIGGER_UPDATE_CHECK_DIALOG"] == "1" (and after
UpdateLogStore.shared.append), replace the single delayed call with logic that
either observes NSWindow/NSApplication window notifications (e.g.,
NSWindow.didBecomeMainNotification or NSApplication.shared.addObserver for
window activation) or polls on the main queue for
NSApplication.shared.mainWindow/visible window, and only invoke
self?.checkForUpdatesInDialog(nil) once a main/visible window exists; keep using
the same DispatchQueue/main context and preserve the 0.25s fallback timing if
desired.
Comment on lines
+5872
to
+5875
| @objc func checkForUpdatesInDialog(_ sender: Any?) { | ||
| updateViewModel.overrideState = nil | ||
| updateController.checkForUpdatesInDialog() | ||
| } |
There was a problem hiding this comment.
Other manual-check callers still bypass this dialog helper.
The split is only partial right now: the provided snippets in Sources/ContentView.swift:10445-10453 and Sources/Update/UpdateDriver.swift:130-145 still call checkForUpdates(nil). Those user-triggered and retry-from-error paths will keep showing the inline pill instead of Sparkle's dialog, so the entry-point behavior from issue #2028 is still inconsistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 5872 - 5875, The ContentView and
UpdateDriver callers still call checkForUpdates(nil) and bypass the dialog
helper; update those call sites to use the dialog entry path: clear any override
by setting updateViewModel.overrideState = nil (or ensure the view model state
is cleared) and invoke AppDelegate's checkForUpdatesInDialog (or call
updateController.checkForUpdatesInDialog()) instead of checkForUpdates(nil) so
user-triggered and retry-from-error flows consistently present Sparkle's dialog;
locate and replace the calls to checkForUpdates(nil) in the methods referenced
(Sources/ContentView.swift and Sources/Update/UpdateDriver.swift) to call the
dialog helper or ensure overrideState is nil before invoking
checkForUpdatesInDialog.
Comment on lines
+78
to
+80
| let releaseNotesSummary = (env["CMUX_UI_TEST_RELEASE_NOTES_SUMMARY"] ?? | ||
| "Important fixes, improved reliability, and small workflow polish.") | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) |
There was a problem hiding this comment.
Sanitize CDATA terminators in release-notes text before XML interpolation.
If the summary contains ]]>, Line 93 produces malformed XML and the appcast becomes unparsable.
🔧 Proposed fix
- let releaseNotesSummary = (env["CMUX_UI_TEST_RELEASE_NOTES_SUMMARY"] ??
+ let releaseNotesSummary = (env["CMUX_UI_TEST_RELEASE_NOTES_SUMMARY"] ??
"Important fixes, improved reliability, and small workflow polish.")
.trimmingCharacters(in: .whitespacesAndNewlines)
+ let safeReleaseNotesSummary = releaseNotesSummary
+ .replacingOccurrences(of: "]]>", with: "]]]]><![CDATA[>")
@@
- <description sparkle:format="plain-text"><![CDATA[\(releaseNotesSummary)]]></description>
+ <description sparkle:format="plain-text"><![CDATA[\(safeReleaseNotesSummary)]]></description>Also applies to: 93-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Update/UpdateTestURLProtocol.swift` around lines 78 - 80, The
releaseNotesSummary string may contain the CDATA terminator "]]>", which will
break XML when interpolated; before using releaseNotesSummary in XML (and
similarly for the values used at the locations referenced around lines 93-94),
replace every occurrence of "]]>" with "]]]]><
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Notes
Summary by CodeRabbit
Refactor
Tests