Skip to content

Remove skipif_macos_bug from tests that pass on Darwin. - #4204

Open
bogwi wants to merge 4 commits into
danvi/feat/add-macos-e2e-to-mac-cifrom
danvi/fix/remove-skipif-macos-bug-from-green-tests
Open

bogwi wants to merge 4 commits into
danvi/feat/add-macos-e2e-to-mac-cifrom
danvi/fix/remove-skipif-macos-bug-from-green-tests

Conversation

@bogwi

@bogwi bogwi commented Sep 17, 2026

Copy link
Copy Markdown
Member
  1. Remove skipif_macos_bug from tests that pass on Darwin.

  2. (9bc0b60) Removed the sqlite-vec Darwin skip and the matching skipif_macos / skipif_aarch64 on TestStandaloneComponents, then marked test_replay.py, test_store.py, and test_stream.py with macos_ci so Mac self-hosted-tests now runs those 246 tests (they were hosted-only before).

@bogwi
bogwi added this pull request to stack #4205 September 17, 2026 14:31
@bogwi
bogwi marked this pull request as ready for review September 17, 2026 14:32
@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
6032 1 6031 106
View the top 1 failed test(s) by shortest run time
dimos.e2e_tests.test_voice_browser::test_hold_to_talk_ships_a_decodable_recording[firefox]
Stack Traces | 22.6s run time
voice_bridge = ('http://127.0.0.1:61606/', [])
fake_mic_page = <Page url='http://127.0.0.1:61606/'>

    def test_hold_to_talk_ships_a_decodable_recording(
        voice_bridge: tuple[str, list[AudioChunk]], fake_mic_page: Page
    ) -> None:
        url, chunks = voice_bridge
        chunks.clear()  # the module fixture is shared across both engines
        fake_mic_page.goto(url)
        mic = fake_mic_page.get_by_test_id("chat-audio_in-mic")
        # Enabled == transport connected; the manifest already placed the panel.
        expect(mic).to_be_enabled(timeout=120_000)
        expect(mic).to_have_attribute("data-state", "idle")
    
        mic.hover()
        fake_mic_page.mouse.down()
>       expect(mic).to_have_attribute("data-state", "recording", timeout=15_000)
E       AssertionError: Locator expected to have attribute 'recording'
E       Actual value: arming 
E       Call log:
E         - Expect "to_have_attribute" with timeout 15000ms
E         - waiting for get_by_test_id("chat-audio_in-mic")
E       
E       Aria snapshot:
E       - button "hold to talk": talk

chunks     = []
fake_mic_page = <Page url='http://127.0.0.1:61606/'>
mic        = <Locator frame=<Frame name= url='http://127.0.0.1:61606/'> selector='internal:testid=[data-testid="chat-audio_in-mic"s]'>
url        = 'http://127.0.0.1:61606/'
voice_bridge = ('http://127.0.0.1:61606/', [])

dimos/e2e_tests/test_voice_browser.py:104: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

Safe to merge; all exercised newly enabled test paths passed.

What we checked:

  • Ran the worker-skip-removal-run.sh to capture paired before/after test runs; both captures show 13 tests collected and 13 passed with exit code 0, and a non-failing ResourceWarning appeared that is unrelated to skip-marker removal. T-Rex
  • Executed the skipif-macos-bug-targets.sh and collected Pub/Sub output before and separate RPC output after to verify behavior. T-Rex
  • Compared the full test suite across base revision 3100524 and PR HEAD f0ea7ac; both showed identical results for reactive and relay/forkserver tests, and the full changed-file execution completed with exit status 0. T-Rex

Summary

  • Removes obsolete Darwin skip markers from selected worker coordination, PubSub, RPC, reactive utility, and dynamic relay tests.
  • Focused checks passed for worker deployment and lifecycle paths, PubSub and RPC transport grids, reactive timeout handling, and relay forkserver deployment.

Merge safety

Safe to merge. The newly enabled test coverage completed successfully with no affected files requiring changes.

Reviews (1) · Last reviewed commit: "Remove skipif_macos_bug from tests that ..."

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 17, 2026
@bogwi
bogwi force-pushed the danvi/fix/remove-skipif-macos-bug-from-green-tests branch from f0ea7ac to 56c4897 Compare September 18, 2026 02:46
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Sep 18, 2026
@bogwi
bogwi force-pushed the danvi/fix/remove-skipif-macos-bug-from-green-tests branch from 9bc0b60 to fc4284f Compare September 18, 2026 03:55
@bogwi
bogwi force-pushed the danvi/fix/remove-skipif-macos-bug-from-green-tests branch from ddef902 to 76a8ade Compare September 18, 2026 05:54
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 18, 2026
if TYPE_CHECKING:
from dimos.memory.store.base import Store

pytestmark = pytest.mark.macos_ci

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.

Are these due to extensions not being loaded? If so, I've fixed this in my PR by using the uv version of Python, rather than the system one.

@bogwi bogwi Sep 18, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

no, these are not skips, these are adds with clear intent - add to mac ci

@bogwi
bogwi requested a review from Dreamsorcerer September 18, 2026 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport release/0.0.14 PlzReview ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants