ci: make the unit workflow green (gate live-Ollama/Docker tests, fix two card-dependent assertions) - #15
Merged
Conversation
… fix two card-dependent assertions With grimoire public, CI's install step finally passes and the test job ran for the first time; it surfaced four kinds of failure, none caused by the code under test: - tests/api/test_context_info_introspection.py and tests/api/test_external_rag_integration.py create LLMCore with the default (Ollama) provider and need a live server. New `requires_ollama` marker on both modules; tests/conftest.py auto-skips marked tests when no Ollama answers (OLLAMA_HOST honoured); the CI `-m` selection excludes the marker explicitly. - test_sandbox_integration::test_run_with_sandbox_creates_and_cleans_up needs a Docker daemon (SandboxIntegration.initialize()); marked `docker`, which CI already excludes. - test_deepinfra_provider::test_get_max_context_length_from_registry hard-coded 163840 for deepseek-ai/DeepSeek-V3; the model-card refresh (#12) took DeepInfra's live value (128000). The test now asserts the provider returns whatever the registry card says. - test_huggingface_provider::test_known_model_fallback_table was order-dependent: it expected the static fallback table (131072) but a builtin card (128000) wins whenever an earlier test has loaded the registry singleton. The registry lookup is now neutralised in the test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The Ollama marker does not consistently honor OLLAMA_HOST in the external RAG fixture.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Updates CI and tests to handle unavailable Ollama/Docker environments and avoid brittle model-card assertions.
Changes:
- Adds Ollama availability detection and CI marker exclusion.
- Marks Docker-dependent sandbox tests.
- Makes provider assertions registry-aware and order-independent.
| File | Summary |
|---|---|
tests/test_huggingface_provider.py |
Isolates fallback-table testing from registry state. |
tests/providers/test_deepinfra_provider.py |
Uses the registry card’s context value. |
tests/conftest.py |
Adds Ollama availability detection and skipping. |
tests/api/test_external_rag_integration.py |
Marks tests as Ollama-dependent. |
tests/api/test_context_info_introspection.py |
Marks tests as Ollama-dependent. |
tests/agents/test_sandbox_integration.py |
Marks the test as Docker-dependent. |
pyproject.toml |
Registers the Ollama marker. |
.github/workflows/ci.yml |
Excludes Ollama tests from CI selection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| import pytest | ||
|
|
||
| pytestmark = pytest.mark.requires_ollama # the llmcore_instance fixture needs a live Ollama |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
With grimoire now public, CI's dependency install passes and the unit test job ran for the first time on
main. It failed for four reasons, none of them bugs in the code under test:tests/api/test_context_info_introspection.py+test_external_rag_integration.pyLLMCore.create()with the default Ollama provider; CI has no Ollamarequires_ollamamarker on both modules;tests/conftest.pyauto-skips them when no server answers (honoursOLLAMA_HOST); CI-mselection excludes the marker explicitlytest_sandbox_integration::test_run_with_sandbox_creates_and_cleans_upSandboxIntegration.initialize()needs a Docker daemondocker(already excluded in CI)test_deepinfra_provider::test_get_max_context_length_from_registry163840; the card refresh (#12) took DeepInfra's live value128000test_huggingface_provider::test_known_model_fallback_tableVerification (local)
OLLAMA_HOSTpointing at a dead port: the two API modules report 48 skipped; with the CI-mstring: 26 deselected / 0 collected for the marked module.tests/test_huggingface_provider.py tests/providers/test_deepinfra_provider.py+ the sandbox test (Docker present locally): 75 passed.🤖 Generated with Claude Code