Skip to content

Read pusher ghost test sources as UTF-8#7753

Closed
tianmind-studio wants to merge 1 commit into
BasedHardware:mainfrom
tianmind-studio:codex/pusher-ghost-test-windows
Closed

Read pusher ghost test sources as UTF-8#7753
tianmind-studio wants to merge 1 commit into
BasedHardware:mainfrom
tianmind-studio:codex/pusher-ghost-test-windows

Conversation

@tianmind-studio

Copy link
Copy Markdown
Contributor

Summary

  • read pusher.py and transcribe.py test source snapshots with explicit UTF-8 encoding
  • avoid Windows locale-dependent Path.read_text() failures during test_pusher_ghost_connections.py collection when the default code page is GBK

Testing

  • python -m pytest tests\unit\test_pusher_ghost_connections.py -q -> 43 passed
  • python -m black --line-length 120 --skip-string-normalization tests\unit\test_pusher_ghost_connections.py --check
  • python -m py_compile tests\unit\test_pusher_ghost_connections.py
  • git diff --check origin/main...HEAD

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds encoding='utf-8' to every Path.read_text() call in test_pusher_ghost_connections.py so that reading pusher.py and transcribe.py source snapshots no longer depends on the OS default code page, fixing Windows test-collection failures under locales like GBK.

  • All 21 bare read_text() calls (used for both AST parsing and string-pattern assertions) are uniformly updated to read_text(encoding='utf-8').
  • No test logic, assertions, or fixture setup is altered — this is a pure encoding-hygiene fix with no functional impact on the tests themselves.

Confidence Score: 5/5

Safe to merge — only adds explicit UTF-8 encoding to file reads with no test-logic changes.

Every bare read_text() call has been updated consistently, the fix is correct and complete, and no assertions or test structure were modified.

No files require special attention.

Important Files Changed

Filename Overview
backend/tests/unit/test_pusher_ghost_connections.py Adds explicit encoding='utf-8' to all 21 Path.read_text() calls that read pusher.py and transcribe.py source snapshots, fixing Windows test-collection failures when the system code page is not UTF-8 (e.g. GBK).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[pytest collects test_pusher_ghost_connections.py] --> B[Path.read_text called on pusher.py / transcribe.py]
    B -->|Before fix - no encoding arg| C{OS default code page}
    C -->|UTF-8 Linux/macOS| D[OK]
    C -->|GBK / CP1252 Windows| E[UnicodeDecodeError — test collection fails]
    B -->|After fix - encoding='utf-8'| F[Always reads as UTF-8]
    F --> G[Tests pass on all platforms]
Loading

Reviews (1): Last reviewed commit: "Read pusher ghost connection test source..." | Re-trigger Greptile

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Backend test stabilization (Windows/UTF-8/optional-dep isolation) — approve only per policy.

Copy link
Copy Markdown
Contributor Author

Closing this in favor of #7797.

#7797 carries the same Windows UTF-8 source-read fix for test_pusher_ghost_connections.py, is based on the newer main, and has the clearer _read_source() helper/validation notes. Keeping only the newer PR should make the review queue easier to process.

@github-actions

Copy link
Copy Markdown
Contributor

Hey @tianmind-studio 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

  • Project standards — Ensuring consistency across the codebase
  • User needs — Making sure changes align with what our users need
  • Code best practices — Maintaining code quality and maintainability
  • Project direction — Keeping aligned with our roadmap and vision

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community! 💜

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