wenjin272 commented on code in PR #292:
URL: https://github.com/apache/flink-agents/pull/292#discussion_r2498597548
##########
python/flink_agents/integrations/chat_models/tests/test_ollama_chat_model.py:
##########
@@ -32,12 +32,15 @@
)
from flink_agents.plan.tools.function_tool import FunctionTool, from_callable
+# Mark all tests in this module as ollama tests
+pytestmark = pytest.mark.ollama
Review Comment:
This should be `pytest.mark.ollama`.
##########
python/flink_agents/integrations/embedding_models/tests/test_openai_embedding_model.py:
##########
@@ -25,14 +25,18 @@
OpenAIEmbeddingModelSetup,
)
+# Mark all tests in this module as integration tests
+pytestmark = pytest.mark.integration
+
test_model = os.environ.get("TEST_EMBEDDING_MODEL", "text-embedding-3-small")
api_key = os.environ.get("TEST_API_KEY")
@pytest.mark.skipif(api_key is None, reason="TEST_API_KEY is not set")
[email protected]
Review Comment:
Have marked all tests in this module above, this decorator maybe not needed.
##########
python/flink_agents/integrations/embedding_models/tests/test_openai_embedding_model.py:
##########
@@ -25,14 +25,18 @@
OpenAIEmbeddingModelSetup,
)
+# Mark all tests in this module as integration tests
+pytestmark = pytest.mark.integration
+
test_model = os.environ.get("TEST_EMBEDDING_MODEL", "text-embedding-3-small")
api_key = os.environ.get("TEST_API_KEY")
@pytest.mark.skipif(api_key is None, reason="TEST_API_KEY is not set")
[email protected]
def test_openai_embedding_model() -> None: # noqa: D103
connection = OpenAIEmbeddingModelConnection(
- name="openai", api_key=api_key
+ name="openai", api_key=api_key or "fake-key"
Review Comment:
If this test is not skipped, the api_key must not be None, the `or
"fake-key"` maybe useless.
##########
python/flink_agents/integrations/embedding_models/local/tests/test_ollama_embedding_model.py:
##########
@@ -58,9 +61,12 @@
)
def test_ollama_embedding_setup() -> None:
"""Test embedding functionality with OllamaEmbeddingModelSetup."""
+ # Use longer timeout for embedding in CI environment (slower resources)
+ request_timeout = 120.0 if os.environ.get("CI") else 30.0
Review Comment:
This request timeout could also happen in local environment, so can uniform
to 120.0
##########
python/flink_agents/integrations/vector_stores/chroma/tests/test_chroma_vector_store.py:
##########
@@ -128,9 +135,9 @@ def get_resource(name: str, resource_type: ResourceType) ->
Resource:
name="chroma_vector_store",
embedding_model="mock_embeddings",
collection="test_collection",
- api_key=api_key,
- tenant=tenant,
- database=database,
+ api_key=api_key or "fake-key",
Review Comment:
ditto
##########
.github/workflows/ci.yml:
##########
@@ -88,8 +88,67 @@ jobs:
uses: astral-sh/setup-uv@v4
with:
version: "latest"
- - name: Run Python Tests
+ - name: Run Python Tests (excluding Integration)
run: tools/ut.sh -p
+ env:
+ PYTEST_SKIP_MARKERS: "integration"
+
+ ollama_integration_tests:
Review Comment:
Maybe this can be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
