Concisely describe the motivation
Currently tests often have many layers of decorators. For example,
@ti.require(ti.extension.assertion)
@ti.all_archs_with(debug=True)
@ti.must_throw(RuntimeError)
def test_assert_minimal():
ti.set_gdb_trigger(False)
@ti.kernel
def func():
assert 0
func()
In practice, it's really hard to arrange the decorators in the correct order. Actually, the order shown above doesn't work correctly. The current implementation is not easy to debug.
The combination itself is limited. For example, there is all_archs_with(debug=True) but there is no host_arch_with.
Describe the solution you'd like (if any)
It would be good to have a unified ti.test decorator so that the above test can be written as
@ti.test(archs=all, require=ti.extension.assertion, debug=True, gdb_trigger=False, must_throw=RuntimeError)
def test_assert_minimal():
@ti.kernel
def func():
assert 0
func()
Additional comments
The current ti.test function should probably be renamed or removed.
This design should be thoroughly discussed before implementation.
Agree...
Just FYI, we don't need @ti.must_throw(RuntimeError) at all, the pytest.raises can do that as well:
@ti.require(ti.extension.assertion)
@ti.all_archs_with(debug=True, gdb_trigger=False)
def test_assert_minimal():
@ti.kernel
def func():
assert 0
with pytest.raises(RuntimeError):
func()
the
pytest.raisescan do that as well:
Cool, that works. Thanks. The refactoring is still needed IMHO.
Also FYI: pytest.fixture can do the work of ti.all_archs_with and ti.require as well...
pytest is really a professional package in test, we should learn how to make use of it, instead of writing our ad-hoc decorators.
I believe all the problems we encounter here could be already meet by many other python people and pytest have systematically solutions for them, WDYT :)
In fact, if we make use of pytest.fixture, we can figure out which arch failed test. e.g.:
Before:
python/taichi/misc/test.py::test_this PASSED
python/taichi/misc/test.py::test_that FAILED # Well... but which arch??
After:
python/taichi/misc/test.py::test_this[x64] PASSED
python/taichi/misc/test.py::test_that[x64] PASSED
python/taichi/misc/test.py::test_this[opengl] PASSED
python/taichi/misc/test.py::test_that[opengl] FAILED # Oh! It's only OpenGL's fault!
Hi! @Rullec, the infrastructure is quite complete here. It's time to change the @ti.all_archs to @ti.test(...)! Would you like help me? We may use the same workflow as rullec-field did. For documentation about @ti.test(...), see #1475.
Nice, I would be happy to try this one. Thanks for your recommendation.
@Rullec Please do replacement in these files: tests/python/test_***.py.
Open PR one by one, e.g.: [test] Use @ti.test in test files starting with [a-c].
Since tests are invisible to end-users, we're OK to directly merge these PRs into master, no rullec-test required, WDYT?
@ti.all_archs # old API
```py
@ti.test() # new API!
```py
@ti.all_archs_excluding(ti.opengl) # old API
```py
@ti.test(exclude=ti.opengl) # new API!
```py
@ti.all_archs # old API
@ti.require(ti.extension.data64) # old-fashion multi-decorator design
```py
@ti.test(require=ti.extension.data64) # new API!
```py
@ti.must_throw(RuntimeError) # don't replace yet, let someone (likely @archibate) take care of them in another PR
@ti.all_archs
def test_XXX(): # all functions starting with `test_`, please replace my decorator!
@ti.all_archs
def _test_XXX(): # if the name starts with `_test_`, don't replace, let @archibate fix these non-trivial ones
@ti.all_archs
def some_other_even_strange_name_XXX(): # don't replace, let @archibate fix these non-trivial ones
Thanks! I have been familiared with this workflow.
Please give me some more time to investigate them carefully and have some PRs!
Since tests are invisible to end-users, we're OK to directly merge these PRs into
master, norullec-testrequired, WDYT?
We should directly merge into master. It's fine if we use multiple PRs for the replacement.
Most helpful comment
@Rullec Please do replacement in these files:
tests/python/test_***.py.Open PR one by one, e.g.:
[test] Use @ti.test in test files starting with [a-c].Since tests are invisible to end-users, we're OK to directly merge these PRs into
master, norullec-testrequired, WDYT?Decorator replace rule:
```py
@ti.test() # new API!
```py
@ti.test(exclude=ti.opengl) # new API!
or equivalently: @ti.test(exclude=[ti.opengl])
```py
@ti.test(require=ti.extension.data64) # new API!
Some functions you don't need to replace: