gromero commented on code in PR #10865:
URL: https://github.com/apache/tvm/pull/10865#discussion_r844665108
##########
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:
@Mousius Thanks for the suggestions! All of them are good improvements. I
specially like the one for using a fixture + a generator for preparing the test
environment and cleaning it up all at once. But there is a caveat: fixtures
don't play well with `@pytest.mark.parametrize`. This is by design afaik.
In the code above `missing_file`, `broken_symlink`, and `fake_directory`
won't reach out the scope of `test_tvmc_compile_invalid_input()` as `str`, but
as a `function`, i.e. the returned value of the fixture is not passed to
`test_tvmc_compile_invalid_input` , but instead the fixture itself is
(https://github.com/pytest-dev/pytest/issues/349). And unfortunately a fixture
can't be called directly allowing something like:
`@pytest.mark.parametrize("invalid_input", [missing_file(),
broken_symlink(), fake_directory()])`
There might be some other "magic" workaround, but that caveat is an example
of pytest "shenanigans" that annoys me. So sometimes I tend to do more "direct"
tests, although that for sure goes against the pytest paradigms...
Anyway, I think the best solution would be to use "pytest-lazy-fixture" to
implement your suggestion (https://github.com/tvorog/pytest-lazy-fixture). This
requires `pytest-lazy-fixture` to get installed into the CI images. I've pushed
(to this PR) a working v2 using `lazy_fixture` that gives me the following
result:
```
gromero@amd:~/git/tvm$ pytest -vvv
tests/python/driver/tvmc/test_command_line.py
[04:45:59] /home/gromero/git/tvm/src/target/target_kind.cc:163: Warning:
Unable to detect CUDA version, default to "-arch=sm_20" instead
enabled targets: llvm
pytest marker:
====================================================================================
test session starts
=====================================================================================
platform linux -- Python 3.9.5, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 --
/usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/gromero/git/tvm
plugins: lazy-fixture-0.6.3
collected 7 items
tests/python/driver/tvmc/test_command_line.py::test_tvmc_cl_workflow PASSED
[ 14%]
tests/python/driver/tvmc/test_command_line.py::test_tvmc_compile_file_check[missing_file]
PASSED
[ 28%]
tests/python/driver/tvmc/test_command_line.py::test_tvmc_compile_file_check[broken_symlink]
PASSED
[ 42%]
tests/python/driver/tvmc/test_command_line.py::test_tvmc_compile_file_check[fake_directory]
PASSED
[ 57%]
tests/python/driver/tvmc/test_command_line.py::test_tvmc_tune_file_check[missing_file]
PASSED
[ 71%]
tests/python/driver/tvmc/test_command_line.py::test_tvmc_tune_file_check[broken_symlink]
PASSED
[ 85%]
tests/python/driver/tvmc/test_command_line.py::test_tvmc_tune_file_check[fake_directory]
PASSED
[100%]
======================================================================================
warnings summary
======================================================================================
tests/python/driver/tvmc/test_command_line.py::test_tvmc_cl_workflow
/home/gromero/.local/lib/python3.9/site-packages/tensorflow/python/autograph/impl/api.py:22:
DeprecationWarning: the imp module is deprecated in favour of importlib; see
the module's documentation for alternative uses
import imp
tests/python/driver/tvmc/test_command_line.py::test_tvmc_cl_workflow
/home/gromero/.local/lib/python3.9/site-packages/xgboost/training.py:18:
UserWarning: Old style callback is deprecated. See:
https://xgboost.readthedocs.io/en/latest/python/callbacks.html
warnings.warn(f'Old style callback is deprecated. See: {link}',
UserWarning)
-- Docs: https://docs.pytest.org/en/stable/warnings.html
===============================================================================
7 passed, 2 warnings in 8.66s
================================================================================
```
But it will fail in the CI.
In the v2 I've also decided to keep a `test_tvmc_compile_file_check` and a
`test_tvmc_tune_file_check` because in the future the `FILE` in `tune` can
accept other inputs beyond a regular file and so the error message can
~~check~~ change and become different than in `compile`. I also avoided
defining `missing_file` as a generator, so I defined it as a simple function
since there's no env. preparation etc.
wdyt? Should we move on with using `pytest-lazy-fixture`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
