From 3a32e0af576b42cab890539393b345f5aeb2a66a Mon Sep 17 00:00:00 2001 From: David Yastremsky Date: Wed, 28 Feb 2024 15:48:15 -0800 Subject: [PATCH 01/10] Add unit tests for CLI --- .../perf_analyzer/genai-pa/tests/test_cli.py | 81 +++++++++++++++++-- 1 file changed, 74 insertions(+), 7 deletions(-) diff --git a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py index cddbb70f9..9d31e8eea 100644 --- a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py +++ b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py @@ -24,14 +24,81 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +import io +import sys + import pytest from genai_pa.main import run -# NOTE: Placeholder -class TestHelp: - @pytest.mark.parametrize("arg", ["-h", "--help"]) - def test_help(self, arg): - args = [arg] - with pytest.raises(SystemExit): - run(args) +class TestCLIArguments: + @pytest.mark.parametrize( + "arg, expected_output", + [ + (["-h"], "CLI to profile LLMs and Generative AI models with Perf Analyzer"), + ( + ["--help"], + "CLI to profile LLMs and Generative AI models with Perf Analyzer", + ), + ], + ) + def test_help_arguments_output_and_exit(self, arg, expected_output, capsys): + combined_args = ["-m", "test_model"] + arg + + with pytest.raises(SystemExit) as exc_info: + run(combined_args) + + # Confirm the exit was successful + assert exc_info.value.code == 0 + + # Confirm the message is correct + captured = capsys.readouterr() + assert expected_output in captured.out + + @pytest.mark.parametrize( + "arg, expected_output", + [ + (["-b", "2"], "batch_size=2"), + (["--batch-size", "2"], "batch_size=2"), + (["--concurrency", "3"], "concurrency_range='3'"), + (["--max-threads", "4"], "max_threads=4"), + ( + ["--profile-export-file", "text.txt"], + "profile_export_file=PosixPath('text.txt')", + ), + (["--request-rate", "1.5"], "request_rate_range='1.5'"), + (["--service-kind", "triton"], "service_kind='triton'"), + (["--service-kind", "openai"], "service_kind='openai'"), + # TODO: Remove streaming from implementation. It is invalid with HTTP. + # (["--streaming"], "Streaming=True"), + (["--version"], "version=True"), + (["-u", "test_url"], "u='test_url'"), + (["--url", "test_url"], "u='test_url'"), + ], + ) + def test_arguments_output(self, arg, expected_output, capsys): + combined_args = ["-m", "test_model"] + arg + # Redirect stdout and stderr to capture output + original_stdout, original_stderr = sys.stdout, sys.stderr + sys.stdout, sys.stderr = io.StringIO(), io.StringIO() + + try: + # Call the run function + # TODO: Un-comment the below when the perf analyzer call works. + # with pytest.raises(SystemExit): + run(combined_args) + finally: + # Restore stdout and stderr + captured_stdout, captured_stderr = ( + sys.stdout.getvalue(), + sys.stderr.getvalue(), + ) + sys.stdout, sys.stderr = original_stdout, original_stderr + + assert expected_output in captured_stdout + assert "" == captured_stderr # Assuming no error, adjust if needed + + def test_arguments_model_not_provided(self): + with pytest.raises(SystemExit) as exc_info: + run() + assert exc_info.value.code != 0 From 1d14134f50e26ec41dfe2a8817bef738ddbd7f49 Mon Sep 17 00:00:00 2001 From: David Yastremsky Date: Wed, 28 Feb 2024 15:50:24 -0800 Subject: [PATCH 02/10] Update comment --- src/c++/perf_analyzer/genai-pa/tests/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py index 9d31e8eea..ac3eccabe 100644 --- a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py +++ b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py @@ -84,7 +84,7 @@ def test_arguments_output(self, arg, expected_output, capsys): try: # Call the run function - # TODO: Un-comment the below when the perf analyzer call works. + # TODO: Un-comment the below when the perf analyzer call works for all the tests. # with pytest.raises(SystemExit): run(combined_args) finally: From 8c17b757ceb982e15e847b9bf93d759de5c8d170 Mon Sep 17 00:00:00 2001 From: David Yastremsky Date: Wed, 28 Feb 2024 15:57:41 -0800 Subject: [PATCH 03/10] Remove unnecessary check --- src/c++/perf_analyzer/genai-pa/tests/test_cli.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py index ac3eccabe..6efeb709e 100644 --- a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py +++ b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py @@ -84,8 +84,6 @@ def test_arguments_output(self, arg, expected_output, capsys): try: # Call the run function - # TODO: Un-comment the below when the perf analyzer call works for all the tests. - # with pytest.raises(SystemExit): run(combined_args) finally: # Restore stdout and stderr From 394731ddb1e2bd0c8cb9c2c530ce1b60dd91210d Mon Sep 17 00:00:00 2001 From: David Yastremsky Date: Wed, 28 Feb 2024 16:36:17 -0800 Subject: [PATCH 04/10] For help test, do not need model name --- src/c++/perf_analyzer/genai-pa/tests/test_cli.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py index 6efeb709e..c58603522 100644 --- a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py +++ b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py @@ -43,10 +43,8 @@ class TestCLIArguments: ], ) def test_help_arguments_output_and_exit(self, arg, expected_output, capsys): - combined_args = ["-m", "test_model"] + arg - with pytest.raises(SystemExit) as exc_info: - run(combined_args) + run(arg) # Confirm the exit was successful assert exc_info.value.code == 0 From 26f976a74001b23e9d9de2c05b56af35d3d7ee7e Mon Sep 17 00:00:00 2001 From: David Yastremsky Date: Wed, 28 Feb 2024 17:13:46 -0800 Subject: [PATCH 05/10] Add pytest as req, use parser for unit tests --- src/c++/perf_analyzer/genai-pa/pyproject.toml | 1 + .../perf_analyzer/genai-pa/tests/test_cli.py | 37 +++++++------------ 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/c++/perf_analyzer/genai-pa/pyproject.toml b/src/c++/perf_analyzer/genai-pa/pyproject.toml index f6bfc850c..9e707fa1f 100644 --- a/src/c++/perf_analyzer/genai-pa/pyproject.toml +++ b/src/c++/perf_analyzer/genai-pa/pyproject.toml @@ -47,6 +47,7 @@ keywords = [] requires-python = ">=3.8,<4" dependencies = [ "numpy", + "pytest", "rich" ] diff --git a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py index c58603522..ab50b15fb 100644 --- a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py +++ b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py @@ -28,6 +28,7 @@ import sys import pytest +from genai_pa import parser from genai_pa.main import run @@ -43,13 +44,13 @@ class TestCLIArguments: ], ) def test_help_arguments_output_and_exit(self, arg, expected_output, capsys): - with pytest.raises(SystemExit) as exc_info: - run(arg) + with pytest.raises(SystemExit) as excinfo: + _ = parser.parse_args(arg) - # Confirm the exit was successful - assert exc_info.value.code == 0 + # Check that the exit was successful + assert excinfo.value.code == 0 - # Confirm the message is correct + # Capture that the correct message was displayed captured = capsys.readouterr() assert expected_output in captured.out @@ -75,26 +76,16 @@ def test_help_arguments_output_and_exit(self, arg, expected_output, capsys): ], ) def test_arguments_output(self, arg, expected_output, capsys): - combined_args = ["-m", "test_model"] + arg - # Redirect stdout and stderr to capture output - original_stdout, original_stderr = sys.stdout, sys.stderr - sys.stdout, sys.stderr = io.StringIO(), io.StringIO() - - try: - # Call the run function - run(combined_args) - finally: - # Restore stdout and stderr - captured_stdout, captured_stderr = ( - sys.stdout.getvalue(), - sys.stderr.getvalue(), - ) - sys.stdout, sys.stderr = original_stdout, original_stderr + combined_args = ["--model", "test_model"] + arg + _ = parser.parse_args(combined_args) - assert expected_output in captured_stdout - assert "" == captured_stderr # Assuming no error, adjust if needed + # Capture that the correct message was displayed + captured = capsys.readouterr() + assert expected_output in captured.out def test_arguments_model_not_provided(self): with pytest.raises(SystemExit) as exc_info: - run() + parser.parse_args() + + # Check that the exit was unsuccessful assert exc_info.value.code != 0 From 2064515dd37b33e8809ce460c80655b3bc7ae2b0 Mon Sep 17 00:00:00 2001 From: David Yastremsky Date: Wed, 28 Feb 2024 17:14:31 -0800 Subject: [PATCH 06/10] Show discarded return value --- src/c++/perf_analyzer/genai-pa/tests/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py index ab50b15fb..8db90f0f9 100644 --- a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py +++ b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py @@ -85,7 +85,7 @@ def test_arguments_output(self, arg, expected_output, capsys): def test_arguments_model_not_provided(self): with pytest.raises(SystemExit) as exc_info: - parser.parse_args() + _ = parser.parse_args() # Check that the exit was unsuccessful assert exc_info.value.code != 0 From e7b934ef5feb1fcbec98e3e5f373df3aa1f69613 Mon Sep 17 00:00:00 2001 From: David Yastremsky Date: Thu, 29 Feb 2024 09:25:12 -0800 Subject: [PATCH 07/10] Remove unused imports --- src/c++/perf_analyzer/genai-pa/tests/test_cli.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py index 8db90f0f9..9a2e3ef74 100644 --- a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py +++ b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py @@ -24,9 +24,6 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -import io -import sys - import pytest from genai_pa import parser from genai_pa.main import run From 655a506416c5c8d7f116de1908bd1e24990ef748 Mon Sep 17 00:00:00 2001 From: David Yastremsky Date: Thu, 29 Feb 2024 10:38:04 -0800 Subject: [PATCH 08/10] Check correct values for args, nothing printed --- .../perf_analyzer/genai-pa/tests/test_cli.py | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py index 9a2e3ef74..bc103acd4 100644 --- a/src/c++/perf_analyzer/genai-pa/tests/test_cli.py +++ b/src/c++/perf_analyzer/genai-pa/tests/test_cli.py @@ -24,9 +24,10 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +from pathlib import Path + import pytest from genai_pa import parser -from genai_pa.main import run class TestCLIArguments: @@ -52,33 +53,37 @@ def test_help_arguments_output_and_exit(self, arg, expected_output, capsys): assert expected_output in captured.out @pytest.mark.parametrize( - "arg, expected_output", + "arg, expected_attributes", [ - (["-b", "2"], "batch_size=2"), - (["--batch-size", "2"], "batch_size=2"), - (["--concurrency", "3"], "concurrency_range='3'"), - (["--max-threads", "4"], "max_threads=4"), + (["-b", "2"], {"batch_size": 2}), + (["--batch-size", "2"], {"batch_size": 2}), + (["--concurrency", "3"], {"concurrency_range": "3"}), + (["--max-threads", "4"], {"max_threads": 4}), ( ["--profile-export-file", "text.txt"], - "profile_export_file=PosixPath('text.txt')", + {"profile_export_file": Path("text.txt")}, ), - (["--request-rate", "1.5"], "request_rate_range='1.5'"), - (["--service-kind", "triton"], "service_kind='triton'"), - (["--service-kind", "openai"], "service_kind='openai'"), + (["--request-rate", "1.5"], {"request_rate_range": "1.5"}), + (["--service-kind", "triton"], {"service_kind": "triton"}), + (["--service-kind", "openai"], {"service_kind": "openai"}), # TODO: Remove streaming from implementation. It is invalid with HTTP. - # (["--streaming"], "Streaming=True"), - (["--version"], "version=True"), - (["-u", "test_url"], "u='test_url'"), - (["--url", "test_url"], "u='test_url'"), + # (["--streaming"], {"streaming": True}), + (["--version"], {"version": True}), + (["-u", "test_url"], {"u": "test_url"}), + (["--url", "test_url"], {"u": "test_url"}), ], ) - def test_arguments_output(self, arg, expected_output, capsys): + def test_arguments_output(self, arg, expected_attributes, capsys): combined_args = ["--model", "test_model"] + arg - _ = parser.parse_args(combined_args) + args = parser.parse_args(combined_args) - # Capture that the correct message was displayed + # Check that the attributes are set correctly + for key, value in expected_attributes.items(): + assert getattr(args, key) == value + + # Check that nothing was printed as a byproduct of parsing the arguments captured = capsys.readouterr() - assert expected_output in captured.out + assert captured.out == "" def test_arguments_model_not_provided(self): with pytest.raises(SystemExit) as exc_info: From 6a00e8853aec5ba820052f206dcc5d9512e41159 Mon Sep 17 00:00:00 2001 From: David Yastremsky Date: Thu, 29 Feb 2024 10:49:16 -0800 Subject: [PATCH 09/10] Remove extra print in CLI --- src/c++/perf_analyzer/genai-pa/genai_pa/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/c++/perf_analyzer/genai-pa/genai_pa/parser.py b/src/c++/perf_analyzer/genai-pa/genai_pa/parser.py index 76cf3f459..e988669c6 100644 --- a/src/c++/perf_analyzer/genai-pa/genai_pa/parser.py +++ b/src/c++/perf_analyzer/genai-pa/genai_pa/parser.py @@ -37,7 +37,6 @@ def prune_args(args: argparse.ArgumentParser) -> argparse.ArgumentParser: """ Prune the parsed arguments to remove args with None or False values. """ - print(args) return argparse.Namespace( **{k: v for k, v in vars(args).items() if v is not None if v is not False} ) @@ -199,6 +198,7 @@ def parse_args(argv=None): parser = argparse.ArgumentParser( prog="genai-pa", description="CLI to profile LLMs and Generative AI models with Perf Analyzer", + formatter_class=argparse.RawDescriptionHelpFormatter, ) parser.set_defaults(func=handler) From 733d6bcaf8e61b6a88dd7f3fb53088b19464db17 Mon Sep 17 00:00:00 2001 From: David Yastremsky Date: Thu, 29 Feb 2024 10:49:52 -0800 Subject: [PATCH 10/10] Remove exrta arg --- src/c++/perf_analyzer/genai-pa/genai_pa/parser.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/c++/perf_analyzer/genai-pa/genai_pa/parser.py b/src/c++/perf_analyzer/genai-pa/genai_pa/parser.py index e988669c6..d4e0497b7 100644 --- a/src/c++/perf_analyzer/genai-pa/genai_pa/parser.py +++ b/src/c++/perf_analyzer/genai-pa/genai_pa/parser.py @@ -198,7 +198,6 @@ def parse_args(argv=None): parser = argparse.ArgumentParser( prog="genai-pa", description="CLI to profile LLMs and Generative AI models with Perf Analyzer", - formatter_class=argparse.RawDescriptionHelpFormatter, ) parser.set_defaults(func=handler)