test: simplify test cases to reduce calls to gpt

.github/workflows/python.yml

@@ -75,7 +75,7 @@ jobs:
           ref: ${{ github.event.pull_request.head.sha }}
           token: ${{ secrets.PAT }}
 
-      - uses: actions/setup-python@v1
+      - uses: actions/setup-python@v4
         with:
           python-version: '3.11.3'
           cache: 'pip' # caching pip dependencies

src/gpt_review/_llama_index.py

@@ -22,10 +22,10 @@
 from llama_index.indices.base import BaseGPTIndex
 from llama_index.storage.storage_context import DEFAULT_PERSIST_DIR
 
-logger = logging.getLogger(__name__)
-
 import gpt_review.constants as C
 
+logger = logging.getLogger(__name__)
+
 
 def _query_index(
     question: str,

src/gpt_review/_openai.py

@@ -96,7 +96,7 @@ def _call_gpt(
             logging.warning("Call to GPT failed due to rate limit, retry attempt %s of %s", retry, C.MAX_RETRIES)
 
             wait_time = int(error.headers["Retry-After"]) if error.headers["Retry-After"] else retry * 10
-            logging.warning(f"Waiting for {wait_time} seconds before retrying.")
+            logging.warning("Waiting for %s seconds before retrying.", wait_time)
 
             time.sleep(wait_time)
 

tests/conftest.py

@@ -52,14 +52,8 @@ def mock_create(
     def from_documents(documents, service_context=None) -> MockIndex:
         return MockIndex()
 
-    class MockRepoReader:
-        def __init__(self, owner, repo, use_parser) -> None:
-            self.owner = owner
-            self.repo = repo
-            self.use_parser = use_parser
-
     def init_mock_reader(self, owner, repo, use_parser) -> None:
-        MockRepoReader(owner=owner, repo=repo, use_parser=use_parser)
+        pass
 
     def mock_load_data_from_branch(self, branch):
         return SimpleDirectoryReader(input_dir=".").load_data()
@@ -69,6 +63,11 @@ def mock_load_data_from_branch(self, branch):
     monkeypatch.setattr("llama_index.GithubRepositoryReader.__init__", init_mock_reader)
     monkeypatch.setattr("llama_index.GithubRepositoryReader._load_data_from_branch", mock_load_data_from_branch)
 
+    def mock_query(self, question) -> MockQueryResponse:
+        return MockQueryResponse()
+
+    monkeypatch.setattr("llama_index.indices.query.base.BaseQueryEngine.query", mock_query)
+
 
 @pytest.fixture
 def mock_github(monkeypatch) -> None:

tests/test_gpt_cli.py

@@ -16,102 +16,88 @@ class CLICase:
     expected_error_code: int = 0
 
 
-ROOT_COMMANDS = [
-    CLICase("--version"),
-    CLICase("--help"),
-]
+@dataclass
+class CLICase1(CLICase):
+    expected_error_code: int = 1
+
+
+@dataclass
+class CLICase2(CLICase):
+    expected_error_code: int = 2
+
+
+SAMPLE_FILE = "src/gpt_review/__init__.py"
+
+QUESTION = "how are you"
 
 WHAT_LANGUAGE = "what programming language is this code written in?"
 
-ASK_COMMANDS = [
-    CLICase("ask --help"),
-    CLICase("ask how are you"),
-    CLICase("ask --fast how are you"),
-    CLICase(f"ask how are you --fast --max-tokens={C.MAX_TOKENS_DEFAULT}"),
-    CLICase(
-        "ask how are you --fast --max-tokens",
-        """usage: gpt ask [-h] [--verbose] [--debug] [--only-show-errors]
+HELP_TEXT = """usage: gpt ask [-h] [--verbose] [--debug] [--only-show-errors]
                [--output {json,jsonc,yaml,yamlc,table,tsv,none}]
                [--query JMESPATH] [--max-tokens MAX_TOKENS]
                [--temperature TEMPERATURE] [--top-p TOP_P]
                [--frequency-penalty FREQUENCY_PENALTY]
                [--presence-penalty PRESENCE_PENALTY]
                <QUESTION> [<QUESTION> ...]
-gpt ask: error: argument --max-tokens: expected one argument
-""",
-        2,
-    ),
+"""
+
+ROOT_COMMANDS = [
+    CLICase("--version"),
+    CLICase("--help"),
+]
+
+ASK_COMMANDS = [
+    CLICase("ask --help"),
+    CLICase(f"ask {QUESTION}"),
+    CLICase(f"ask --fast {QUESTION}"),
     CLICase(
-        "ask how are you --fast --max-tokens 'test'",
-        """usage: gpt ask [-h] [--verbose] [--debug] [--only-show-errors]
-               [--output {json,jsonc,yaml,yamlc,table,tsv,none}]
-               [--query JMESPATH] [--max-tokens MAX_TOKENS]
-               [--temperature TEMPERATURE] [--top-p TOP_P]
-               [--frequency-penalty FREQUENCY_PENALTY]
-               [--presence-penalty PRESENCE_PENALTY]
-               <QUESTION> [<QUESTION> ...]
-gpt ask: error: argument --max-tokens: invalid int value: \"'test'\"
-""",
-        2,
+        f"ask {QUESTION} --fast --max-tokens {C.MAX_TOKENS_DEFAULT} --temperature {C.TEMPERATURE_DEFAULT} --top-p {C.TOP_P_DEFAULT} --frequency-penalty {C.FREQUENCY_PENALTY_DEFAULT} --presence-penalty {C.FREQUENCY_PENALTY_MAX}"
     ),
-    CLICase(
-        f"ask how are you --fast --max-tokens {C.MAX_TOKENS_MIN-1}",
+    CLICase1(
+        f"ask {QUESTION} --fast --max-tokens {C.MAX_TOKENS_MIN-1}",
         f"ERROR: --max-tokens must be a(n) int between {C.MAX_TOKENS_MIN} and {C.MAX_TOKENS_MAX}\n",
-        1,
     ),
-    CLICase(f"ask how are you --fast --temperature {C.TEMPERATURE_DEFAULT}"),
-    CLICase(
-        f"ask how are you --temperature {C.TEMPERATURE_MAX+8}",
+    CLICase1(
+        f"ask {QUESTION} --temperature {C.TEMPERATURE_MAX+8}",
         f"ERROR: --temperature must be a(n) float between {C.TEMPERATURE_MIN} and {C.TEMPERATURE_MAX}\n",
-        1,
     ),
-    CLICase(f"ask how are you --fast --top-p {C.TOP_P_DEFAULT}"),
-    CLICase(
-        f"ask how are you --top-p {C.TOP_P_MAX+3.5}",
+    CLICase1(
+        f"ask {QUESTION} --top-p {C.TOP_P_MAX+3.5}",
         f"ERROR: --top-p must be a(n) float between {C.TOP_P_MIN} and {C.TOP_P_MAX}\n",
-        1,
     ),
-    CLICase(f"ask how are you --fast --frequency-penalty {C.FREQUENCY_PENALTY_DEFAULT}"),
-    CLICase(
-        f"ask how are you --frequency-penalty {C.FREQUENCY_PENALTY_MAX+2}",
+    CLICase1(
+        f"ask {QUESTION} --frequency-penalty {C.FREQUENCY_PENALTY_MAX+2}",
         f"ERROR: --frequency-penalty must be a(n) float between {C.FREQUENCY_PENALTY_MIN} and {C.FREQUENCY_PENALTY_MAX}\n",
-        1,
     ),
-    CLICase(f"ask how are you --fast --presence-penalty {C.PRESENCE_PENALTY_DEFAULT}"),
-    CLICase(
-        f"ask how are you --presence-penalty {C.PRESENCE_PENALTY_MAX+7.7}",
+    CLICase1(
+        f"ask {QUESTION} --presence-penalty {C.PRESENCE_PENALTY_MAX+7.7}",
         f"ERROR: --presence-penalty must be a(n) float between {C.PRESENCE_PENALTY_MIN} and {C.PRESENCE_PENALTY_MAX}\n",
-        1,
-    ),
-    CLICase(f"ask how are you --fast --max-tokens={C.MAX_TOKENS_DEFAULT} --temperature {C.TEMPERATURE_DEFAULT}"),
-    CLICase(f"ask how are you --fast --max-tokens={C.MAX_TOKENS_DEFAULT} --top-p {C.TOP_P_DEFAULT}"),
-    CLICase(
-        f"ask how are you --fast --max-tokens {C.MAX_TOKENS_MAX+1} --top-p {C.TOP_P_DEFAULT}",
-        f"ERROR: --max-tokens must be a(n) int between {C.MAX_TOKENS_MIN} and {C.MAX_TOKENS_MAX}\n",
-        1,
-    ),
-    CLICase(
-        f"ask how are you --fast --frequency-penalty {C.FREQUENCY_PENALTY_DEFAULT} --presence-penalty {C.PRESENCE_PENALTY_DEFAULT}"
-    ),
-    CLICase(
-        f"ask how are you --fast --frequency-penalty {C.FREQUENCY_PENALTY_MAX+3} --presence-penalty {C.PRESENCE_PENALTY_MAX+7.7}",
-        f"ERROR: --frequency-penalty must be a(n) float between {C.FREQUENCY_PENALTY_MIN} and {C.FREQUENCY_PENALTY_MAX}\n",
-        1,
     ),
-    CLICase(
-        f"ask how are you --fast --max-tokens={C.MAX_TOKENS_DEFAULT} --temperature {C.TEMPERATURE_DEFAULT} --frequency-penalty {C.FREQUENCY_PENALTY_DEFAULT} --presence-penalty {C.FREQUENCY_PENALTY_MAX}"
+    CLICase2(
+        f"ask {QUESTION} --fast --max-tokens",
+        f"""{HELP_TEXT}
+gpt ask: error: argument --max-tokens: expected one argument
+""",
     ),
-    CLICase(
-        f"""ask how are you --fast --max-tokens {C.MAX_TOKENS_DEFAULT} --top-p {C.TOP_P_DEFAULT} --frequency-penalty {C.FREQUENCY_PENALTY_DEFAULT} --presence-penalty {C.FREQUENCY_PENALTY_MAX}"""
+    CLICase2(
+        f"ask {QUESTION} --fast --max-tokens 'test'",
+        f"""{HELP_TEXT}
+gpt ask: error: argument --max-tokens: invalid int value: \"'test'\"
+""",
     ),
-    CLICase("github review --help"),
-    CLICase("github review"),
+    CLICase(f"ask --files {SAMPLE_FILE} --files {SAMPLE_FILE} what programming language is this code written in?"),
+    CLICase(f"ask --fast -f {SAMPLE_FILE} what programming language is this code written in?"),
     CLICase(f"ask --files src/gpt_review/__init__.py --files src/gpt_review/__init__.py {WHAT_LANGUAGE}"),
     CLICase(f"ask --fast -f src/gpt_review/__init__.py {WHAT_LANGUAGE}"),
     CLICase(f"ask --fast -d src/gpt_review --recursive --hidden --required-exts .py {WHAT_LANGUAGE}"),
     CLICase(f"ask --fast -repo microsoft/gpt-review {WHAT_LANGUAGE}"),
 ]
 
+GITHUB_COMMANDS = [
+    CLICase("github review --help"),
+    CLICase("github review"),
+]
+
 GIT_COMMANDS = [
     CLICase("git commit --help"),
     # CLICase("git commit"),
@@ -125,14 +111,21 @@ class CLICase:
     CLICase("review diff --diff tests/mock.diff --config tests/config.summary.test.yml"),
 ]
 
-ARGS = ROOT_COMMANDS + ASK_COMMANDS + GIT_COMMANDS + REVIEW_COMMANDS
+ARGS = ROOT_COMMANDS + ASK_COMMANDS + GIT_COMMANDS + GITHUB_COMMANDS + REVIEW_COMMANDS
+ARGS_DICT = {arg.command: arg for arg in ARGS}
+
+MODULE_COMMANDS = [
+    CLICase("python -m gpt --version"),
+    CLICase("python -m gpt_review --version"),
+]
+MODULE_DICT = {arg.command: arg for arg in MODULE_COMMANDS}
 
 
 def gpt_cli_test(command: CLICase) -> None:
     os.environ["GPT_ASK_COMMANDS"] = "1"
 
     sys.argv[1:] = command.command.split(" ")
-
+    exit_code = -1
     try:
         exit_code = cli()
     except SystemExit as e:
@@ -141,28 +134,7 @@ def gpt_cli_test(command: CLICase) -> None:
         assert exit_code == command.expected_error_code
 
 
-@pytest.mark.parametrize("command", ARGS)
-@pytest.mark.integration
-def test_int_gpt_cli(command: CLICase) -> None:
-    """Test gpt commands from CLI file"""
-    gpt_cli_test(command)
-
-
-@pytest.mark.parametrize(
-    "command",
-    ARGS,
-)
-def test_gpt_cli(command: CLICase, mock_openai: None) -> None:
-    gpt_cli_test(command)
-
-
-@pytest.mark.parametrize("command", ARGS)
-@pytest.mark.cli
-def test_cli_gpt_cli(command: CLICase) -> None:
-    """Test gpt commands from installed CLI"""
-
-    command_array = f"gpt {command.command}".split(" ")
-
+def cli_test(command, command_array) -> None:
     result = subprocess.run(
         command_array,
         stdout=subprocess.PIPE,
@@ -173,24 +145,31 @@ def test_cli_gpt_cli(command: CLICase) -> None:
     assert result.returncode == command.expected_error_code
 
 
-MODULE_COMMANDS = [
-    CLICase("python -m gpt --version"),
-    CLICase("python -m gpt_review --version"),
-]
+@pytest.mark.parametrize("command", ARGS_DICT.keys())
+@pytest.mark.cli
+def test_cli_gpt_cli(command: str) -> None:
+    """Test gpt commands from installed CLI"""
+    command_array = f"gpt {ARGS_DICT[command].command}".split(" ")
+
+    cli_test(ARGS_DICT[command], command_array)
 
 
-@pytest.mark.parametrize("command", MODULE_COMMANDS)
+@pytest.mark.parametrize("command", MODULE_DICT.keys())
 @pytest.mark.cli
-def test_cli_gpt_module(command: CLICase) -> None:
+def test_cli_gpt_module(command: str) -> None:
     """Test running cli as module"""
+    command_array = MODULE_DICT[command].command.split(" ")
 
-    command_array = command.command.split(" ")
+    cli_test(MODULE_DICT[command], command_array)
 
-    result = subprocess.run(
-        command_array,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-        check=False,
-    )
 
-    assert result.returncode == command.expected_error_code
+@pytest.mark.parametrize("command", ARGS_DICT.keys())
+def test_gpt_cli(command: str, mock_openai: None) -> None:
+    gpt_cli_test(ARGS_DICT[command])
+
+
+@pytest.mark.parametrize("command", ARGS_DICT.keys())
+@pytest.mark.integration
+def test_int_gpt_cli(command: str) -> None:
+    """Test gpt commands from CLI file"""
+    gpt_cli_test(ARGS_DICT[command])