Skip to content

Account avatar tests write real pixels into their test images - #12596

Closed
ejc3 wants to merge 1 commit into
manaflow-ai:mainfrom
ejc3:fix/avatar-test-fixture-color
Closed

ejc3 wants to merge 1 commit into
manaflow-ai:mainfrom
ejc3:fix/avatar-test-fixture-color

Conversation

@ejc3

@ejc3 ejc3 commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Four account avatar tests fail because their test images are fully transparent: hostedRequestRendersPictureThroughSharedRenderer, circularImageCenterCropsWideSourceAtRequestedScale, circularImageCenterCropsTallSource and circularImageClipsCornersInsideTheBitmap. Each expects green pixels and reads zeros.

Both test helpers fill a bitmap with color.usingColorSpace(.deviceRGB). For NSColor.green, NSBitmapImageRep.setColor rejects that converted color ("Unrecognized colorspace number -1") and writes nothing. A standalone probe on macOS 26.6 arm64 reads back 0,0,0,0 for that call and green for NSColor(deviceRed: 0, green: 1, blue: 0, alpha: 1). The renderer then correctly falls back to the placeholder symbol.

The helpers now rebuild each color from its components with NSColor(deviceRed:green:blue:alpha:) before writing it. Product code is unchanged.

Testing

  • Ran StackAccountAvatarViewTests and StackAccountAvatarImageLoaderTests on an EC2 Mac (Xcode 26.6, macOS 15.7) through scripts/ci/run-app-host-xcodebuild.sh, the way CI runs app-host tests. At main 4638e5b1ea plus the compile fixes in Fix the compile errors that keep main and its unit tests from building #12584, both suites fail on the four tests above. With this change on the same base, both suites pass.
  • A standalone probe on macOS 26.6 arm64 reads back 0,0,0,0 after setColor(NSColor.green.usingColorSpace(.deviceRGB)) and green after setColor(NSColor(deviceRed: 0, green: 1, blue: 0, alpha: 1)).

Demo Video

Not applicable. The change only affects unit tests.

Review Trigger (Copy/Paste as PR comment)

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

Checklist

  • I tested the change locally
  • I added or updated tests for behavior changes
  • I updated docs/changelog if needed
  • I requested bot reviews after my latest commit (copy/paste block above or equivalent)
  • All code review bot comments are resolved
  • All human review comments are resolved

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes four account avatar tests that failed because their test images were fully transparent. Test helpers now rebuild each color with NSColor(deviceRed:green:blue:alpha:) before writing it to the bitmap, so the expected green pixels are actually written instead of being rejected by setColor and left transparent. Product code is unchanged.

Written for commit 5582f6e. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved color handling when generating avatar image test data, ensuring PNG output is written correctly across supported color formats.
    • This helps ensure avatar rendering validation accurately reflects expected results.
  • Tests

    • Strengthened coverage for account avatar images and avatar views.
    • No user-facing behavior changes are included in this update.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 7b0efe9b-057b-4944-bba3-ea49a6b6bb11

📥 Commits

Reviewing files that changed from the base of the PR and between 211b8bb and 5582f6e.

📒 Files selected for processing (2)
  • cmuxTests/StackAccountAvatarImageLoaderTests.swift
  • cmuxTests/StackAccountAvatarViewTests.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

Changes

PNG test color encoding

Layer / File(s) Summary
Rebuild device RGB colors
cmuxTests/StackAccountAvatarImageLoaderTests.swift, cmuxTests/StackAccountAvatarViewTests.swift
The pngData helpers extract RGBA components from converted colors, rebuild explicit device RGB NSColor values, and pass them to setColor. One helper documents the conversion behavior.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to 5582f

The PR narrowly repairs avatar test PNG generation, and no remaining merge-blocking behavior is identified.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: account avatar test images now contain actual pixel data.
Description check ✅ Passed The description explains what changed, why it changed, how it was tested, and why a demo video is not applicable. It includes the required review trigger and checklist. Remaining unchecked items accur…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed PASS. The authoritative diff changes only two files under cmuxTests/: private pngData test helpers. It adds no production Swift declarations, service protocols, models, Sendable reference types, o…
Cmux Swift Blocking Runtime ✅ Passed PASS: The authoritative diff changes only cmuxTests/StackAccountAvatarImageLoaderTests.swift and cmuxTests/StackAccountAvatarViewTests.swift. It replaces a test bitmap color argument with reconstr…
Cmux Browser Automation Off-Main ✅ Passed PASS. The PR changes only two account-avatar test helpers. The diff updates NSBitmapImageRep.setColor calls and does not touch Sources/TerminalController.swift, `ControlCommandExecutionPolicy.swif…
Cmux Expensive Synchronous Load ✅ Passed PASS. The reviewed range changes only two files under cmuxTests, and both belong to the cmuxTests.xctest target. The diff only rebuilds NSColor values before NSBitmapImageRep.setColor. It adds…
Cmux Cache Substitution Correctness ✅ Passed PASS: The authoritative PR diff changes only cmuxTests/StackAccountAvatarImageLoaderTests.swift and cmuxTests/StackAccountAvatarViewTests.swift. The changes rebuild NSColor values before `NSBitm…
Cmux No Hacky Sleeps ✅ Passed PASS. The authoritative diff changes only two Swift test files under cmuxTests/. It rebuilds NSColor values before NSBitmapImageRep.setColor; it does not add sleeps, timers, polling, delays, or …
Cmux Algorithmic Complexity ✅ Passed PASS: The PR changes only cmuxTests/StackAccountAvatarImageLoaderTests.swift and cmuxTests/StackAccountAvatarViewTests.swift. The modified code is test-only bitmap fixture setup. Its nested pixel …
Cmux Swift Concurrency ✅ Passed PASS: The pull request changes only two XCTest helper implementations in cmuxTests. The diff replaces NSBitmapImageRep.setColor input construction with explicit `NSColor(deviceRed:green:blue:alpha…
Cmux Swift @Concurrent ✅ Passed PASS. The PR changes only synchronous private pngData helpers in two test files. The diff adds no async, await, nonisolated, or @concurrent declarations and does not change any call sites or…
Cmux Swift Package Boundaries ✅ Passed PASS. The authoritative diff changes only cmuxTests/StackAccountAvatarImageLoaderTests.swift and cmuxTests/StackAccountAvatarViewTests.swift. The changes are private PNG test helpers that rebuild …
Cmux Swiftpm Lockfiles ✅ Passed PASS: The authoritative PR diff changes only cmuxTests/StackAccountAvatarImageLoaderTests.swift and cmuxTests/StackAccountAvatarViewTests.swift. The patch only rebuilds NSColor values before `NS…
Cmux Swift Logging ✅ Passed The pull request changes only two files under cmuxTests/. The added lines rebuild NSColor values before NSBitmapImageRep.setColor; they add no print, debugPrint, dump, NSLog, file loggin…
Cmux User-Facing Error Privacy ✅ Passed PASS: The reviewed range changes only two cmuxTests/*Tests.swift files. The diff updates test bitmap helpers and adds a developer-only comment. It adds no production behavior, user-facing errors, al…
Cmux Full Internationalization ✅ Passed PASS. The review-scoped diff changes only two cmuxTests/*.swift test helpers. The added lines rebuild NSColor values and add developer-only comments; they do not add or change user-facing text, lo…
Cmux Swiftui State Layout ✅ Passed PASS. The reviewed diff changes only two AppKit test helper methods in cmuxTests. It reconstructs NSColor values before NSBitmapImageRep.setColor. It adds no SwiftUI view, ObservableObject, `@…
Cmux Architecture Rethink ✅ Passed PASS: The PR changes only two test-local pngData helpers. The diff replaces an invalid NSBitmapImageRep.setColor input with an explicit NSColor(deviceRed:green:blue:alpha:) value. It adds no sle…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The pull request changes only two files under cmuxTests/: test-only pngData helpers. The diff rebuilds NSColor values before NSBitmapImageRep.setColor; it does not add or materially chan…
Cmux Source Artifacts ✅ Passed The PR changes only two existing Swift test source files under cmuxTests/. The diff adds hand-written test-helper code and comments; it adds no logs, media, temporary directories, caches, build outp…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The pull request changes only cmuxTests/StackAccountAvatarImageLoaderTests.swift and cmuxTests/StackAccountAvatarViewTests.swift. No Swift file under a production Sources/ path changed. The addi…
Cmux No Ambient Global State ✅ Passed PASS. The pull request changes only two files under cmuxTests/, not production Swift code. The diff updates the body of existing private static func pngData helpers inside test structs. It adds no…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@ejc3
ejc3 marked this pull request as ready for review September 14, 2026 18:20
The avatar test helpers fill a bitmap with
`color.usingColorSpace(.deviceRGB)`. For `NSColor.green` that conversion
gives a color `NSBitmapImageRep.setColor` rejects ("Unrecognized colorspace
number -1"), so every pixel stays transparent. The renderer then correctly
falls back to the placeholder symbol, and the tests that expect a green
picture fail.

Rebuild each color from its components with `NSColor(deviceRed:green:blue:alpha:)`
before writing it. A standalone probe shows the old call leaves 0,0,0,0 and
the new one writes green.
@teamleaderleo

Copy link
Copy Markdown
Collaborator

Thanks for tracking this one down! Main picked up the same fix in c8bfb58, so I'm closing this as done. Really appreciate all the test cleanup :)

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.

2 participants