[GitHub] [tvm] Mousius commented on a diff in pull request #10865: [TVMC] tune: Check if FILE exists (#10608)

[GitBox <gi...@apache.org>]

Mousius commented on code in PR #10865:
URL: https://github.com/apache/tvm/pull/10865#discussion_r843688721


##########
tests/python/driver/tvmc/test_command_line.py:
##########
@@ -56,3 +58,59 @@ def test_tvmc_cl_workflow(keras_simple, tmpdir_factory):
     run_args = run_str.split(" ")[1:]
     _main(run_args)
     assert os.path.exists(output_path)
+
+
+# Test if tvmc FILE option is checked and, if invalid, is handled correctly.

Review Comment:
   Thanks for adding the tests @gromero, I have a few suggestions to improve them:
   
   * Create an explicit test for each part of tvmc tested
   * Use a temporary directory outside of the tree to keep the tree itself tidy 
   * Split the file creation into `pytest.fixture`s which should mean we don't need the clean up logic in the test itself or the branching to figure out what to remove
   * Use `pytest.parameterize` to generate the test cases rather than loops inside the tests
   
   Something like:
   
   ```python
   @pytest.fixture
   def missing_file(tmp_path):
     missing_file_name = "missing_file_as_invalid_input.tfite"
     yield missing_file_name
   
   @pytest.fixture
   def broken_symlink(tmp_path):
       broken_symlink = "broken_symlink_as_invalid_input.tflite"
       os.symlink("non_existing_file", tmp_path /  broken_symlink)
       yield broken_symlink
       os.unlink(tmp_path / broken_symlink)
   
   @pytest.fixture
   def fake_directory(tmp_path):
       dir_as_invalid = "dir_as_invalid_input.tflite"
       os.mkdir(tmp_path / dir_as_invalid)
       yield dir_as_invalid
       shutil.rmtree(tmp_path / dir_as_invalid)
   
   @pytest.mark.parametrize("invalid_input", [missing_file, broken_symlink, fake_directory])
   def test_tvmc_compile_invalid_input(capsys, missing_file, broken_symlink, fake_directory, invalid_input):
           compile_cmd = f"tvmc compile --target 'c' {invalid_input}"
           run_arg = compile_cmd.split(" ")[1:]
   
           _main(run_arg)
   
           captured = capsys.readouterr()
           expected_err = (
               f"Error: Input file '{invalid_input}' doesn't exist, "
               "is a broken symbolic link, or a directory.\n"
           )
           on_assert_error = f"'tvmc compile' failed to check invalid FILE: {invalid_input}"
           assert captured.err == expected_err, on_assert_error
   ```
   
   What do you think? 😸 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org