Skip to content

Conversation

@sarpit2907
Copy link

This change replaces the hard dependency on minio during test collection
with pytest.importorskip, so tests are skipped gracefully when the optional
dependency is not installed.

This improves the contributor experience and prevents pytest collection
failures due to missing optional test dependencies.

@github-actions github-actions bot added the enhancement Indicates new improvements label Jan 7, 2026
{"id": 2, "key": 6},
{"id": 2, "key": 5},
{"id": 1, "key": 5},
{"id": 2, "key": 5},
Copy link
Member

Choose a reason for hiding this comment

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

We will not need to fix this since .fetch does not preserve order in this case. This is related to #1242 We have addressed this in DataJoint 2.0 (in review now).

@dimitri-yatsenko
Copy link
Member

Thanks for raising this! DataJoint 2.0 (on the docs-2.0-migration branch) addresses test dependency isolation more comprehensively:

Current 2.0 Approach

  1. Automatic test markers: Tests are auto-marked with requires_mysql or requires_minio based on fixture usage—no manual decorators needed.

  2. Lazy container imports: The testcontainers imports are inside fixture functions, not at module level:

    @pytest.fixture(scope="session")
    def minio_container():
        if USE_EXTERNAL_CONTAINERS:
            yield None
            return
        from testcontainers.minio import MinioContainer  # Import only when needed
        ...
  3. Run unit tests without Docker:

    pytest -m "not requires_mysql"
  4. External container support for CI or local docker-compose:

    DJ_USE_EXTERNAL_CONTAINERS=1 pytest

Additional Improvement

Your suggestion to use pytest.importorskip would complement this nicely for the edge case where someone runs integration tests without testcontainers installed. We could add:

@pytest.fixture(scope="session")
def minio_container():
    if USE_EXTERNAL_CONTAINERS:
        yield None
        return
    
    MinioContainer = pytest.importorskip("testcontainers.minio").MinioContainer
    ...

This would gracefully skip instead of failing during fixture setup.

Would you like to submit a PR targeting the docs-2.0-migration branch with this enhancement?

@dimitri-yatsenko
Copy link
Member

Hi @sarpit2907, thank you for this contribution!

We're closing this PR because DataJoint 2.0 uses testcontainers which automatically manages MySQL and MinIO containers during tests. This means:

  • No manual container setup required
  • No missing dependency issues - testcontainers handles everything
  • Tests run consistently across all environments

The contributor experience improvement you aimed for is achieved through a different approach - testcontainers provides a cleaner solution than skipping tests.

See the updated test infrastructure in PR #1312.

@dimitri-yatsenko
Copy link
Member

Closing in favor of DataJoint 2.0's testcontainers approach. Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Indicates new improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants