Skip to content

Conversation

@Mx-Iris
Copy link
Collaborator

@Mx-Iris Mx-Iris commented Aug 16, 2025

Close #454

@Mx-Iris Mx-Iris requested review from Kyle-Ye and Copilot August 16, 2025 15:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an NSHostingView deallocation issue on macOS by properly closing windows and ensuring proper cleanup behavior in tests. The changes address issue #454 where NSHostingView was not being deallocated correctly, which was causing inconsistent test results between macOS and other platforms.

  • Removes macOS-specific conditional compilation that was working around the deallocation issue
  • Adds proper window cleanup by setting isReleasedWhenClosed = false and explicitly calling close() on macOS
  • Updates test expectations to be consistent across all platforms

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
AppearanceActionModifierCompatibilityTests.swift Removes macOS-specific workaround and unifies test expectations across platforms
PlatformHostingControllerHelper.swift Adds proper window cleanup logic for macOS to ensure NSHostingView deallocation

#expect(Helper.result == "AADD")
#endif

await #expect(Helper.result == "AADD")
Copy link

Copilot AI Aug 16, 2025

Choose a reason for hiding this comment

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

The await keyword is used with #expect which is not an async function. This will cause a compilation error. Remove the await keyword.

Suggested change
await #expect(Helper.result == "AADD")
#expect(Helper.result == "AADD")

Copilot uses AI. Check for mistakes.
import OpenSwiftUITestsSupport

@MainActor
struct AppearanceActionModifierCompatibilityTests {
Copy link

Copilot AI Aug 16, 2025

Choose a reason for hiding this comment

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

The @MainActor attribute was removed from this test struct. According to the coding guidelines, test structures should maintain proper isolation when dealing with UI components. Since this test involves appearance actions which typically require main actor isolation, consider whether this removal is intentional or if it might cause concurrency issues.

Copilot uses AI. Check for mistakes.
@Kyle-Ye Kyle-Ye merged commit 61f55f3 into main Aug 16, 2025
7 checks passed
@Kyle-Ye Kyle-Ye deleted the bugfix/macos_ui_testing branch August 16, 2025 16: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.

NSHostingView leak issue

3 participants