From bc12644091df94455a363475c3651a3b59d7b8aa Mon Sep 17 00:00:00 2001 From: Kumaran Rajendhiran Date: Fri, 17 Jan 2025 05:23:30 +0000 Subject: [PATCH] Use pytest marker instead of --skip-docker flag --- .github/workflows/core-test.yml | 2 +- scripts/test-skip-llm.sh | 14 +++++++++++++- test/coding/test_commandline_code_executor.py | 7 +++++-- .../coding/test_embedded_ipython_code_executor.py | 13 +++++++++++-- test/conftest.py | 15 --------------- test/test_code_utils.py | 7 +++++-- website/docs/contributor-guide/tests.mdx | 4 ++-- 7 files changed, 37 insertions(+), 25 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/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 8b8645dbc5..14b6a58a57 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -12,8 +12,6 @@ import autogen -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" @@ -21,19 +19,6 @@ reason = "requested to skip" -# Registers command-line options like '--skip-docker' 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-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_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 555b963e32..7202e02dcf 100644 --- a/website/docs/contributor-guide/tests.mdx +++ b/website/docs/contributor-guide/tests.mdx @@ -34,7 +34,7 @@ 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. +- `-m "not docker"`: Skips tests that explicitly require Docker. - `-m "not redis"`: Skips tests that require a Redis server. @@ -42,7 +42,7 @@ See [here](https://github.com/ag2ai/ag2/blob/main/notebook/contributing.md#testi 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: