Skip to content

Add arm to test matrix#2268

Open
Dreamsorcerer wants to merge 6 commits into
mainfrom
Dreamsorcerer-patch-2
Open

Add arm to test matrix#2268
Dreamsorcerer wants to merge 6 commits into
mainfrom
Dreamsorcerer-patch-2

Conversation

@Dreamsorcerer
Copy link
Copy Markdown
Collaborator

Fixes #2263.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@Dreamsorcerer Dreamsorcerer marked this pull request as ready for review May 27, 2026 16:31
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 27, 2026

Greptile Summary

This PR adds an ARM64 hosted runner (ubuntu-24.04-arm) to the test matrix and works around a broken sqlite-vec aarch64 wheel (ships a 32-bit ELF binary) by introducing a skipif_no_sqlite_vec pytest marker that skips the affected tests on aarch64 and macOS.

  • The CI matrix OS values are changed from the bare string ubuntu (previously concatenated with -latest) to the full runner label ubuntu-latest, and condition checks are updated to startsWith(matrix.os, 'ubuntu') so the new ARM runner label is also matched.
  • A new _no_sqlite_vec() helper and skipif_no_sqlite_vec marker are added in the root conftest.py; matching imperative pytest.skip() guards are placed inside the sqlite_store and sqlite_spy_session fixtures, and class/method-level markers are applied across test_registry.py and test_store.py.
  • The condition platform.machine() == "aarch64" or platform.system() == "Darwin" is duplicated verbatim in dimos/memory2/conftest.py and dimos/memory2/test_store.py rather than imported from a shared location.

Confidence Score: 4/5

Safe to merge — the ARM runner is properly gated by skip logic, and the existing x86-64 test matrix is unaffected.

The CI refactor is clean and the skip mechanism is correctly wired through both the marker path and the fixture path. The only blemish is the platform-check condition being copy-pasted into three files instead of living in one place, but all three copies are identical today so there is no immediate divergence risk.

dimos/memory2/conftest.py and dimos/memory2/test_store.py both duplicate the platform skip condition independently of dimos/conftest.py.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Adds ubuntu-24.04-arm to the test matrix, switches OS names to full runner labels, and updates condition checks to use startsWith() to match the new naming pattern.
dimos/conftest.py Adds _no_sqlite_vec() helper and registers the skipif_no_sqlite_vec marker, wired into pytest_collection_modifyitems for collection-time skipping.
dimos/memory2/conftest.py Adds _SKIP_SQLITE_VEC constant and an imperative pytest.skip() guard inside the sqlite_store fixture; the same condition logic is duplicated from dimos/conftest.py rather than shared.
dimos/memory2/test_registry.py Adds skipif_no_sqlite_vec markers to TestRegistryStore, TestStoreReopen, and two individual methods in TestComponentSerialization that instantiate SQLite-backed stores.
dimos/memory2/test_store.py Adds _SKIP_SQLITE_VEC constant (again duplicating the condition), uses it in sqlite_spy_session fixture, and marks TestStandaloneComponents with skipif_no_sqlite_vec.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CI Matrix Job] --> B{matrix.os}
    B -->|ubuntu-latest| C[x86-64 Ubuntu Runner]
    B -->|ubuntu-24.04-arm| D[ARM64 Ubuntu Runner]
    C --> E{startsWith os ubuntu?}
    D --> E
    E -->|Yes| F[apt-get install portaudio19-dev]
    F --> G[Run pytest]
    G --> H{platform.machine == aarch64?}
    H -->|Yes| I[skipif_no_sqlite_vec active]
    H -->|No| J[All tests run]
    I --> K[sqlite-vec tests SKIPPED]
    I --> L[Other tests RUN]
    J --> M[All tests including sqlite-vec RUN]
Loading

Reviews (1): Last reviewed commit: "Include macos" | Re-trigger Greptile

Comment thread dimos/memory2/test_store.py
Copy link
Copy Markdown
Member

@leshy leshy left a comment

Choose a reason for hiding this comment

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

actually @Dreamsorcerer "skip if no sqlite vec" is a bit unclear on where this happens,

can we have standardized skip tag per OS? like this would have skip_arm tag? so that we know which OS can cover which functionality

@Dreamsorcerer
Copy link
Copy Markdown
Collaborator Author

can we have standardized skip tag per OS? like this would have skip_arm tag? so that we know which OS can cover which functionality

We can, but this needs to be skipped in macos, so I was merging this skip. Can change to adding 2 skip markers if you prefer?

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.

ARM CI

2 participants