Python Shirt tests don't accurately test command line args

[curiouskiwi]

For each of the command line checks, we are looking for a non-zero exit code, but that will happen if the program crashes with an IndexError when it tries to access argv[1] or argv[2] for example, or with a PIL.UnidentifiedImageError if it tries to open a non-suitable image file.

Our checks currently can't distinguish between a purposeful sys.exit("invalid usage") and one of those errors. Hence green smiles even if the code does not check for such things.

@CarterZenke related to: #196

[Makaze]

Are you aiming to find out if they deliberately called sys.exit() in all these cases? I thought I could solve this by making a second dependency test for zero exit given correct arguments, but I'm not sure it it does what you're suggesting.

[bxngyn]

I see how the current check50 script has leniency stemming from only checking non-zero exit codes, not specific errors. Some solutions I thought of were to (1) regex the submission file to manually check whether the student purposefully raises certain errors, or (2) instead of passing the check for any non-zero exit code, we could compare the stderr message from the student's submission to the stderr message expected per the specification. For (2), my thought process was that, to achieve the various and specific error messages, a student must've implemented purposeful checks. However, they could also just print these messages to stderr and pass a check or two. For (1), I think using regex would overcomplicate the testing script quite a bit, even though it would be the most robust way to check for purposeful error handling. I'm personally leaning more towards option (2) and making clear in the specification that the student's error message for each case must match.

[curiouskiwi]

This issue is also present in the other problems in Week 6 (Lines, Pizza, Scourgify) that use command line arguments. All (except the "too many args" test in Lines) will pass the tests for command line args with no corresponding code.

[AkshajSinghal]

I’ve submitted a PR that adds explicit checks for CLI argument validation in the shirt problem.

Specifically, it covers, incorrect number of arguments, invalid input/output file extensions, mismatched file extensions

Previously, these cases were not explicitly tested, which could allow incorrect implementations to pass check50.
Would appreciate any feedback!