Add CLI unit tests for GenAI-PA CLI

[dyastremsky]

Built off of Ryan's test files, here are the first few unit tests for the GenAI-PA CLI. These test all the current CLI args.

To run, you can call pip install . and pytest tests/ in perf_analyzer/genai_pa.

This is the draft for CLI unit testing. If we want to write unit tests that actually check output, I'll need to mock the server (or copy over MA's mock) so that we can test that the subprocess call returns the expected output. Given the scope of the CLI at present, I think that may be overkill and we should be testing that the subprocess call is correct, then have end-to-end testing to show that it works as expected with a Triton server running.

[rmccorm4]

Nice use of parameterize!

I think this handling is fine for getting a "speed-of-light" solution out, but longer term, it may be more maintainable and easy to understand to use the parser directly for this kind of test (assuming I'm interpreting the test correctly).

I haven't tested this would actually work, but I'm thinking something like this:

    from genai_pa import parser
    combined_args = ["-m", "test_model"] + arg
    args = parser.parse_args(combined_args)
    # ... operate checks/assertions on the args object here

Rather than working around stdout/redirecting/parsing it, etc.

[rmccorm4]

Note, all the run() function is doing is calling:

args = parser.parse_args(argv)
args.func()

so if you're just looking for input/parser validation and don't care about running the PA/subcommand for this test, I think this would work.

[debermudez]

If we are confident that the parser is correct, that should work. Just let the parser do the creation of the ArgParser object for you.
I think you just need to pass in a list of comma separated strings to represent the sys args.

[rmccorm4]

To summarize, I'd recommend trying to operate on the parser directly in python objects rather than parsing stdout if it makes sense to do so for this test -- but I wouldn't block the PR approval if you don't think it's worth the time to try right now.

[dyastremsky]

Great idea! Let me give that a try.

[dyastremsky]

Updated! It looks much cleaner, thanks for the feedback.

[rmccorm4]

this is exactly what I was thinking of - nice!!

[rmccorm4]

note: I think some of the expected attributes may change if Elias uses dest like parser.add_argument("--batch-size", ..., dest="b") in a later PR, so these parameters may have to change to {"b": 2} etc.

[debermudez]

I tried to guard against that with metavar but its a good call out.

[rmccorm4]

AFAIK, metavar only modifies the --help output, but won't change the attributes:

import argparse
parser = argparse.ArgumentParser()
parser.add_argument("-b", "--batch-size", default=1, dest="b", metavar="batch_size")
args = parser.parse_args()

for attr in ["b", "batch_size"]:
    print(f'{attr=}: {hasattr(args, attr)=}')

$ python test.py 
attr='b': hasattr(args, attr)=True
attr='batch_size': hasattr(args, attr)=False

[dyastremsky]

Thanks for calling this out and expounding on how metavar works, Ryan! We'll have these tests now, so we should make sure they are still passing after any implementation changes. If not, we can update the tests to match changes in how we want to the CLI designed.

[rmccorm4]

I think it's moot since it looks like the current code isn't using metavar for --batch-size and is instead just mapping it later on before calling subprocess. Just mentioning it in case you got unexpected test failures like this later after a dest change.

dest="u" is being used for --url, but looks like David accounted for that already:

            (["--url", "test_url"], {"u": "test_url"}),