[macOS] Fix DeallocationTests crash with CSS 14.0.0#4477
Open
[macOS] Fix DeallocationTests crash with CSS 14.0.0#4477
Conversation
CSS 14.0.0 NTP JS now triggers an https:// navigation during page initialization, hitting the integration-test navigation guard in Tab.willStart(). Add TestSchemeHandler (http/https mock) to NewTabPageOmnibarActionsHandlerTests, matching the pattern used in AddressBarTests and other integration tests. Task/Issue URL: https://app.asana.com/1/137249556945/project/1202406491309510/task/1214067738586364 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CSS 14.0.0 NTP JS triggers a real https://duckduckgo.com navigation during page init. testWhenLastTabClosed_windowIsDeallocated opened windows with .duckDuckGo without a TestSchemeHandler, hitting the integration-test navigation guard (Tab.willStart fatalError). Use data: URLs as the sibling test testWindowsDeallocation already does. Also revert incorrect change to NewTabPageOmnibarActionsHandlerTests.swift (file is not compiled into any test target). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… issues - Add missing modelId/images args to submitChat calls (signature updated) - Fix testViewAllAiChats_opensNewAIChatTab: was passing .sameTab but asserting 2 tabs (new tab behaviour); correct target is .newTab - Clear firedPixels in tearDown to satisfy test isolation checker Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
26de925 to
8b1fc36
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Task/Issue URL: https://app.asana.com/1/137249556945/project/1202406491309510/task/1214067738586364
Tech Design URL: N/A
CC: N/A
Description
DeallocationTests.testWhenLastTabClosed_windowIsDeallocatedwas navigating tohttps://duckduckgo.com(via.duckDuckGocontent) without aTestSchemeHandler, triggering theTab.willStart()integration-test navigation guard added in Nov 2025 — replaced withdata:URLs, matching the approach already used by the sibling testtestWindowsDeallocation()NewTabPageOmnibarActionsHandlerTests(previously a dangling file not compiled into any target) now compiles: fixedsubmitChatcalls to include the requiredmodelId/imagesparameters, correctedtestViewAllAiChats_opensNewAIChatTabto use.newTabtarget (was.sameTabbut asserting 2 tabs), and addedfiredPixels = []totearDownto satisfy the test isolation checkerTesting Steps
DeallocationTests/testWhenLastTabClosed_windowIsDeallocatedin the Integration Tests target — should pass without SIGTRAPNewTabPageOmnibarActionsHandlerTests— all 9 tests should passImpact and Risks
Impact Level: Low
What could go wrong?
testWhenLastTabClosed_windowIsDeallocatedused.duckDuckGointentionally to test window deallocation with a "real" URL content type — replacing withdata:means it no longer exercises that exact code path, but deallocation behaviour is the same regardless of URL schemeQuality Considerations
Notes to Reviewer
The root crash: CSS 14.0.0 NTP JavaScript triggers a real
https://duckduckgo.comnavigation during page initialisation. Any integration test that creates a window/tab loading the NTP without a registeredTestSchemeHandlerwill hitTab.willStart()'s fatalError. The fix forDeallocationTestsis minimal and matches the existing pattern in the same file.Note
Low Risk
Test-only changes that adjust fixtures and expectations; minimal risk outside potential CI behavior differences due to the new
data:URL loading path.Overview
Fixes macOS integration-test instability by updating
DeallocationTests.testWhenLastTabClosed_windowIsDeallocatedto open windows with adata:URL instead of.duckDuckGo, avoiding real navigation side effects during CI.Also wires
NewTabPageOmnibarActionsHandlerTestsinto the Xcode test target and updates the tests to match current APIs (addsmodelId/imagesargs), corrects theviewAllAiChatsopen target to.newTab, and resetsfiredPixelsintearDownfor test isolation.Reviewed by Cursor Bugbot for commit 8b1fc36. Bugbot is set up for automated code reviews on this repo. Configure here.