And make them part of the blender_test runner. The one exception is blenlib
performance tests, which we don't want to run by default. They remain in their
own executable.
Ref T73268
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- master
- Build Status
Buildable 9404 Build 9404: arc lint + arc unit
Event Timeline
| source/blender/blenlib/tests/performance/BLI_ghash_performance_test.cc | ||
|---|---|---|
| 484 ↗ | (On Diff #27523) | This function was unused (and there was no warning since the compiler flags were different). Since we are testing a 1 and 4 integers already, I guess we wouldn't find additional performance issues with 2 integers. |
Comment Actions
So nice to see more tests being moved into The Big Test Runner :)
| intern/guardedalloc/CMakeLists.txt | ||
|---|---|---|
| 91 ↗ | (On Diff #27523) | This should have bf_intern_guardedalloc and possibly bf_intern_guardedalloc_cpp too. |
| source/blender/blenlib/tests/BLI_delaunay_2d_test.cc | ||
| 129 ↗ | (On Diff #27523) | This looks a bit dodgy to me. IMO disabled tests shouldn't even exist, unless they are clearly marked with a comment that explains why they are disabled, why they're worth keeping around at all, and under which circumstances they should be enabled. This particular change is probably just to avoid some "static function not used" warnings, so that's fine by me. The whole approach to selecting which tests are part of the test suite doesn't seem right to me, though. |
| source/blender/blenlib/tests/BLI_path_util_test.cc | ||
| 5 ↗ | (On Diff #27523) | This can be cleaned up now by adding ../../../source/blender/imbuf to TEST_INC and then just #include "IMB_imbuf.h". Same for the BKE_global.h include below. |
| source/blender/blenlib/tests/performance/CMakeLists.txt | ||
| 25 ↗ | (On Diff #27523) | I think it would be nicer to not add this directory to $INC and use #include "tests/whateverfile.h" instead. |
| source/blender/blenloader/CMakeLists.txt | ||
| 109–110 ↗ | (On Diff #27523) | This should include bf_blenloader, or it may cause linker errors. |
| source/blender/bmesh/CMakeLists.txt | ||
| 214 ↗ | (On Diff #27523) | This should include bf_mesh. |
| source/blender/io/alembic/CMakeLists.txt | ||
| 121 ↗ | (On Diff #27523) | This should include bf_alembic |
| source/blender/io/common/CMakeLists.txt | ||
| 59 ↗ | (On Diff #27523) | Same as above, I would prefer having ../../blenloader on the include path, and then using #include "tests/whatever.h". |
| source/blender/blenlib/tests/BLI_delaunay_2d_test.cc | ||
|---|---|---|
| 129 ↗ | (On Diff #27523) | #if DO_FILE_TESTS was already used in this file, just not for all the functions that needed it. I tried to make the comment above points_from_file_test a bit more clear. We don't currently have automated performance testing so this type of code seems fine to me. |
Comment Actions
Generally fine, indeed very nice to get more of those tests moved to the single binary (and to see ugly stubs to disappear too).
| intern/ffmpeg/CMakeLists.txt | ||
|---|---|---|
| 38 ↗ | (On Diff #27565) | Should be set(TEST_LIB ${TEST_LIB} ${OPENJPEG_LIBRARIES}) I think? Also weird that we do not need includes for openjpeg? But that was already the case in existing code... |
| intern/guardedalloc/CMakeLists.txt | ||
| 95 ↗ | (On Diff #27565) | Do we actually need ${INC} and ${LIB} here? And if yes, wouldn't it be cleaner to explicitly add those to ${TEST_INC}/${TEST_LIB} ? Although this is already done like that in existing code, so I guess it is fine with this patch as well. |
| intern/guardedalloc/CMakeLists.txt | ||
|---|---|---|
| 95 ↗ | (On Diff #27565) | I think that in many cases the tests will need similar #includes as the code under test. Having some extra paths in the test include path doesn't seem that harmful to me. |