From 149a361c276d87eb2ff63d671f592c886da976d2 Mon Sep 17 00:00:00 2001 From: Kumaran Rajendhiran Date: Fri, 17 Jan 2025 13:13:20 +0530 Subject: [PATCH] Use pytest markers instead of --skip-redis and --skip-docker flags (#525) * Use pytest marker instead of --skip-redis flag * Use pytest marker instead of --skip-docker flag --- .github/workflows/core-test.yml | 2 +- scripts/test-skip-llm.sh | 14 +++++++++++++- test/agentchat/test_cache_agent.py | 7 +++++-- test/cache/test_redis_cache.py | 7 ++----- test/coding/test_commandline_code_executor.py | 7 +++++-- .../test_embedded_ipython_code_executor.py | 13 +++++++++++-- test/conftest.py | 19 ------------------- test/test_code_utils.py | 7 +++++-- website/docs/contributor-guide/tests.mdx | 6 +++--- 9 files changed, 45 insertions(+), 37 deletions(-) diff --git a/.github/workflows/core-test.yml b/.github/workflows/core-test.yml index 6de4c95088..ca5ae175a6 100644 --- a/.github/workflows/core-test.yml +++ b/.github/workflows/core-test.yml @@ -95,7 +95,7 @@ jobs: - name: Test with pytest skipping openai and docker tests if: matrix.python-version != '3.10' && matrix.os != 'ubuntu-latest' run: | - bash scripts/test-core-skip-llm.sh --skip-docker + bash scripts/test-core-skip-llm.sh -m "not docker" - name: Coverage with Redis if: matrix.python-version == '3.10' run: | diff --git a/scripts/test-skip-llm.sh b/scripts/test-skip-llm.sh index cc2e3b94cc..acbe5bf410 100755 --- a/scripts/test-skip-llm.sh +++ b/scripts/test-skip-llm.sh @@ -4,4 +4,16 @@ # # SPDX-License-Identifier: Apache-2.0 -bash scripts/test.sh -m "not (openai or gemini or anthropic)" "$@" +base_filter="not (openai or gemini or anthropic)" +args=() +while [[ $# -gt 0 ]]; do + if [[ "$1" == "-m" ]]; then + shift + base_filter="$base_filter and ($1)" + else + args+=("$1") + fi + shift +done + +bash scripts/test.sh -m "$base_filter" "${args[@]}" diff --git a/test/agentchat/test_cache_agent.py b/test/agentchat/test_cache_agent.py index 2661abc32c..61a5040487 100644 --- a/test/agentchat/test_cache_agent.py +++ b/test/agentchat/test_cache_agent.py @@ -14,7 +14,7 @@ from autogen.agentchat import AssistantAgent, UserProxyAgent from autogen.cache import Cache -from ..conftest import Credentials, skip_redis +from ..conftest import Credentials try: from openai import OpenAI # noqa: F401 @@ -28,7 +28,7 @@ except ImportError: skip_redis_tests = True else: - skip_redis_tests = False or skip_redis + skip_redis_tests = False @pytest.mark.openai @@ -85,6 +85,7 @@ def _test_redis_cache(credentials: Credentials): @pytest.mark.openai +@pytest.mark.redis @pytest.mark.skipif(skip_tests or skip_redis_tests, reason="redis not installed OR openai not installed") def test_redis_cache(credentials_gpt_4o_mini: Credentials): _test_redis_cache(credentials_gpt_4o_mini) @@ -92,6 +93,7 @@ def test_redis_cache(credentials_gpt_4o_mini: Credentials): @pytest.mark.skip(reason="Currently not working") @pytest.mark.gemini +@pytest.mark.redis @pytest.mark.skipif(skip_tests or skip_redis_tests, reason="redis not installed OR openai not installed") def test_redis_cache_gemini(credentials_gemini_pro: Credentials): _test_redis_cache(credentials_gemini_pro) @@ -99,6 +101,7 @@ def test_redis_cache_gemini(credentials_gemini_pro: Credentials): @pytest.mark.skip(reason="Currently not working") @pytest.mark.anthropic +@pytest.mark.redis @pytest.mark.skipif(skip_tests or skip_redis_tests, reason="redis not installed OR openai not installed") def test_redis_cache_anthropic(credentials_anthropic_claude_sonnet: Credentials): _test_redis_cache(credentials_anthropic_claude_sonnet) diff --git a/test/cache/test_redis_cache.py b/test/cache/test_redis_cache.py index 3cef95ddab..e135e2d50d 100755 --- a/test/cache/test_redis_cache.py +++ b/test/cache/test_redis_cache.py @@ -20,19 +20,19 @@ skip_redis_tests = True +@pytest.mark.redis +@pytest.mark.skipif(skip_redis_tests, reason="redis not installed") class TestRedisCache(unittest.TestCase): def setUp(self): self.seed = "test_seed" self.redis_url = "redis://localhost:6379/0" - @pytest.mark.skipif(skip_redis_tests, reason="redis not installed") @patch("autogen.cache.redis_cache.redis.Redis.from_url", return_value=MagicMock()) def test_init(self, mock_redis_from_url): cache = RedisCache(self.seed, self.redis_url) self.assertEqual(cache.seed, self.seed) mock_redis_from_url.assert_called_with(self.redis_url) - @pytest.mark.skipif(skip_redis_tests, reason="redis not installed") @patch("autogen.cache.redis_cache.redis.Redis.from_url", return_value=MagicMock()) def test_prefixed_key(self, mock_redis_from_url): cache = RedisCache(self.seed, self.redis_url) @@ -40,7 +40,6 @@ def test_prefixed_key(self, mock_redis_from_url): expected_prefixed_key = f"autogen:{self.seed}:{key}" self.assertEqual(cache._prefixed_key(key), expected_prefixed_key) - @pytest.mark.skipif(skip_redis_tests, reason="redis not installed") @patch("autogen.cache.redis_cache.redis.Redis.from_url", return_value=MagicMock()) def test_get(self, mock_redis_from_url): key = "key" @@ -54,7 +53,6 @@ def test_get(self, mock_redis_from_url): cache.cache.get.return_value = None self.assertIsNone(cache.get(key)) - @pytest.mark.skipif(skip_redis_tests, reason="redis not installed") @patch("autogen.cache.redis_cache.redis.Redis.from_url", return_value=MagicMock()) def test_set(self, mock_redis_from_url): key = "key" @@ -64,7 +62,6 @@ def test_set(self, mock_redis_from_url): cache.set(key, value) cache.cache.set.assert_called_with(f"autogen:{self.seed}:{key}", serialized_value) - @pytest.mark.skipif(skip_redis_tests, reason="redis not installed") @patch("autogen.cache.redis_cache.redis.Redis.from_url", return_value=MagicMock()) def test_context_manager(self, mock_redis_from_url): with RedisCache(self.seed, self.redis_url) as cache: diff --git a/test/coding/test_commandline_code_executor.py b/test/coding/test_commandline_code_executor.py index 5c744bdfb9..a9c3406325 100644 --- a/test/coding/test_commandline_code_executor.py +++ b/test/coding/test_commandline_code_executor.py @@ -22,9 +22,9 @@ from autogen.coding.local_commandline_code_executor import LocalCommandLineCodeExecutor sys.path.append(os.path.join(os.path.dirname(__file__), "..")) -from conftest import MOCK_OPEN_AI_API_KEY, skip_docker +from ..conftest import MOCK_OPEN_AI_API_KEY -if skip_docker or not is_docker_running() or not decide_use_docker(use_docker=None): +if not is_docker_running() or not decide_use_docker(use_docker=None): skip_docker_test = True classes_to_test = [LocalCommandLineCodeExecutor] else: @@ -79,6 +79,7 @@ def test_create_local() -> None: assert executor is config["executor"] +@pytest.mark.docker @pytest.mark.skipif( skip_docker_test, reason="docker is not running or requested to skip docker tests", @@ -252,6 +253,7 @@ def test_local_commandline_code_executor_restart() -> None: # This is kind of hard to test because each exec is a new env +@pytest.mark.docker @pytest.mark.skipif( skip_docker_test, reason="docker is not running or requested to skip docker tests", @@ -265,6 +267,7 @@ def test_docker_commandline_code_executor_restart() -> None: assert result.exit_code == 0 +@pytest.mark.docker @pytest.mark.skipif( skip_docker_test, reason="docker is not running or requested to skip docker tests", diff --git a/test/coding/test_embedded_ipython_code_executor.py b/test/coding/test_embedded_ipython_code_executor.py index 372a84d906..e1e37bdfd6 100644 --- a/test/coding/test_embedded_ipython_code_executor.py +++ b/test/coding/test_embedded_ipython_code_executor.py @@ -14,10 +14,19 @@ import pytest from autogen.agentchat.conversable_agent import ConversableAgent +from autogen.code_utils import ( + decide_use_docker, + is_docker_running, +) from autogen.coding.base import CodeBlock, CodeExecutor from autogen.coding.factory import CodeExecutorFactory -from ..conftest import MOCK_OPEN_AI_API_KEY, skip_docker +from ..conftest import MOCK_OPEN_AI_API_KEY + +if not is_docker_running() or not decide_use_docker(use_docker=None): + skip_docker_test = True +else: + skip_docker_test = False try: from autogen.coding.jupyter import ( @@ -43,7 +52,7 @@ def __init__(self, **kwargs): else: classes_to_test = [EmbeddedIPythonCodeExecutor, LocalJupyterCodeExecutor] - if not skip_docker: + if not skip_docker_test: classes_to_test.append(DockerJupyterExecutor) skip = False diff --git a/test/conftest.py b/test/conftest.py index 4cf958178c..14b6a58a57 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -12,9 +12,6 @@ import autogen -skip_redis = False -skip_docker = False - KEY_LOC = str((Path(__file__).parents[1] / "notebook").resolve()) OAI_CONFIG_LIST = "OAI_CONFIG_LIST" MOCK_OPEN_AI_API_KEY = "sk-mockopenaiAPIkeysinexpectedformatsfortestingonly" @@ -22,22 +19,6 @@ reason = "requested to skip" -# Registers command-line options like '--skip-docker' and '--skip-redis' via pytest hook. -# When these flags are set, it indicates that tests requiring OpenAI or Redis (respectively) should be skipped. -def pytest_addoption(parser: pytest.Parser) -> None: - parser.addoption("--skip-redis", action="store_true", help="Skip all tests that require redis") - parser.addoption("--skip-docker", action="store_true", help="Skip all tests that require docker") - - -# pytest hook implementation extracting command line args and exposing it globally -@pytest.hookimpl(tryfirst=True) -def pytest_configure(config: pytest.Config) -> None: - global skip_redis - skip_redis = config.getoption("--skip-redis", False) - global skip_docker - skip_docker = config.getoption("--skip-docker", False) - - class Credentials: """Credentials for the OpenAI API.""" diff --git a/test/test_code_utils.py b/test/test_code_utils.py index 6d1911c692..dbf93a9491 100755 --- a/test/test_code_utils.py +++ b/test/test_code_utils.py @@ -31,11 +31,11 @@ is_docker_running, ) -from .conftest import Credentials, skip_docker +from .conftest import Credentials here = os.path.abspath(os.path.dirname(__file__)) -if skip_docker or not is_docker_running() or not decide_use_docker(use_docker=None): +if not is_docker_running() or not decide_use_docker(use_docker=None): skip_docker_test = True else: skip_docker_test = False @@ -179,6 +179,7 @@ def scrape(url): assert len(codeblocks) == 1 and codeblocks[0] == ("", "source setup.sh") +@pytest.mark.docker @pytest.mark.skipif(skip_docker_test, reason="docker is not running or requested to skip docker tests") def test_execute_code(use_docker=True): # Test execute code and save the code to a file. @@ -243,6 +244,7 @@ def test_execute_code(use_docker=True): assert isinstance(image, str) +@pytest.mark.docker @pytest.mark.skipif(skip_docker_test, reason="docker is not running or requested to skip docker tests") def test_execute_code_with_custom_filename_on_docker(): with tempfile.TemporaryDirectory() as tempdir: @@ -257,6 +259,7 @@ def test_execute_code_with_custom_filename_on_docker(): assert image == "python:codetest.py" +@pytest.mark.docker @pytest.mark.skipif( skip_docker_test, reason="docker is not running or requested to skip docker tests", diff --git a/website/docs/contributor-guide/tests.mdx b/website/docs/contributor-guide/tests.mdx index 5180ee86e2..7202e02dcf 100644 --- a/website/docs/contributor-guide/tests.mdx +++ b/website/docs/contributor-guide/tests.mdx @@ -34,15 +34,15 @@ See [here](https://github.com/ag2ai/ag2/blob/main/notebook/contributing.md#testi ## Skip flags for tests - `-m`: Used to select or deselect specific groups of tests marked with pytest markers, such as tests for LLM services like OpenAI, Gemini etc. For example, you can mark tests with `@pytest.mark.openai` and then use `-m openai` to run only those tests. -- `--skip-docker`: Skips tests that explicitly require Docker. -- `--skip-redis`: Skips tests that require a Redis server. +- `-m "not docker"`: Skips tests that explicitly require Docker. +- `-m "not redis"`: Skips tests that require a Redis server. **Examples:** To skip tests that require access to LLM services and Docker, run: ```bash -bash scripts/test-core-skip-llm.sh --skip-docker +bash scripts/test-core-skip-llm.sh -m "not docker" ``` To run tests for all LLM services (OpenAI, Gemini, etc.), use: