Skip to content

Remove tofix and add skip_macos#1216

Open
paul-nechifor wants to merge 1 commit intodevfrom
pauln-remove-tofix
Open

Remove tofix and add skip_macos#1216
paul-nechifor wants to merge 1 commit intodevfrom
pauln-remove-tofix

Conversation

@paul-nechifor
Copy link
Contributor

@paul-nechifor paul-nechifor commented Feb 8, 2026

  • The tofix tests actually work (locally and in CI) and are useful because they test CLIP which is used in spatial memory.

@greptile-apps
Copy link

greptile-apps bot commented Feb 8, 2026

Greptile Overview

Greptile Summary

This PR removes the tofix marker from the CLIP image embedding tests and from the default pytest -m exclusion list, and introduces a new skip_macos marker. A new collection hook in dimos/conftest.py attempts to skip any tests marked skip_macos when running on Darwin, so CLIP tests can still be executed in CI/Linux while being disabled on macOS where they are known to fail.

Confidence Score: 3/5

  • This PR is likely safe to merge after addressing the macOS skipping reliability in the conftest hook.
  • Changes are small and localized to pytest configuration/markers, but the new macOS skip behavior is implemented via a collection hook whose registration reliability should be tightened to ensure skip_macos always takes effect on Darwin.
  • dimos/conftest.py

Important Files Changed

Filename Overview
dimos/agents_deprecated/memory/test_image_embedding.py Removes the tofix marker from CLIP embedding tests and adds a class-level skip_macos marker so the whole suite is skipped on macOS.
dimos/conftest.py Adds a pytest_collection_modifyitems hook to skip tests marked skip_macos on Darwin; however, the hook definition is missing the @pytest.hookimpl decorator and can be silently ignored depending on plugin configuration.
pyproject.toml Removes the tofix marker and its exclusion from default pytest addopts, and registers new skip_macos marker; default run will now include previously tofix-marked tests on non-macOS.

Sequence Diagram

sequenceDiagram
    participant Pytest as pytest
    participant Conftest as dimos/conftest.py
    participant Collector as collection phase
    participant Test as test_image_embedding.py

    Pytest->>Conftest: load conftest plugins
    Pytest->>Collector: collect tests/items
    Collector-->>Test: discovers TestImageEmbedding items
    Collector->>Conftest: pytest_collection_modifyitems(config, items)
    alt platform.system() == "Darwin"
        Conftest->>Collector: add skip(reason="skipped on macOS") to items with marker skip_macos
    else non-macOS
        Conftest-->>Collector: no modifications
    end
    Pytest->>Collector: apply -m expression from pyproject addopts
    Collector-->>Pytest: selected items (heavy excluded by default)
    Pytest->>Test: run selected tests

Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

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