[Bf-blender-cvs] [ea137af7520] sybren-usd: Updated for review comments

Sybren A. Stüvel noreply at git.blender.org
Thu Nov 21 10:36:07 CET 2019


Commit: ea137af7520cb6f6ef2769d135b7c304c85569c9
Author: Sybren A. Stüvel
Date:   Thu Nov 14 16:59:38 2019 +0100
Branches: sybren-usd
https://developer.blender.org/rBea137af7520cb6f6ef2769d135b7c304c85569c9

Updated for review comments

- Removed link to CMake docs
- EXTRA_CLI → COMMAND_ARGS
- BlendfileLoadingAbstractTest → BlendfileLoadingBaseTest
- Moved BlendfileLoadingBaseTest into tests/gtests/blenloader
- Added failure message
- Check C++ string with a C++ string function
- Full stop.

===================================================================

M	build_files/cmake/Modules/GTestTesting.cmake
M	tests/gtests/blenloader/CMakeLists.txt
M	tests/gtests/blenloader/blendfile_load_test.cc
R079	tests/gtests/testing/blendfile_loading_test.cc	tests/gtests/blenloader/blendfile_loading_base_test.cc
R092	tests/gtests/testing/blendfile_loading_test.h	tests/gtests/blenloader/blendfile_loading_base_test.h
M	tests/gtests/testing/CMakeLists.txt

===================================================================

diff --git a/build_files/cmake/Modules/GTestTesting.cmake b/build_files/cmake/Modules/GTestTesting.cmake
index 1d5c40e2e85..2a05b92ba3f 100644
--- a/build_files/cmake/Modules/GTestTesting.cmake
+++ b/build_files/cmake/Modules/GTestTesting.cmake
@@ -14,10 +14,9 @@
 
 macro(BLENDER_SRC_GTEST_EX)
   if(WITH_GTESTS)
-    # See https://cmake.org/cmake/help/v3.5/command/cmake_parse_arguments.html
     set(options SKIP_ADD_TEST)
     set(oneValueArgs NAME)
-    set(multiValueArgs SRC EXTRA_LIBS EXTRA_CLI)
+    set(multiValueArgs SRC EXTRA_LIBS COMMAND_ARGS)
     cmake_parse_arguments(ARG "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN} )
 
     set(TARGET_NAME ${ARG_NAME}_test)
@@ -69,7 +68,7 @@ macro(BLENDER_SRC_GTEST_EX)
     if(NOT ARG_SKIP_ADD_TEST)
       add_test(
         NAME ${TARGET_NAME}
-        COMMAND ${TESTS_OUTPUT_DIR}/${TARGET_NAME} ${ARG_EXTRA_CLI}
+        COMMAND ${TESTS_OUTPUT_DIR}/${TARGET_NAME} ${ARG_COMMAND_ARGS}
         WORKING_DIRECTORY ${TEST_INSTALL_DIR})
 
       # Don't fail tests on leaks since these often happen in external libraries
diff --git a/tests/gtests/blenloader/CMakeLists.txt b/tests/gtests/blenloader/CMakeLists.txt
index 2d9d41d520f..83e11b82f6b 100644
--- a/tests/gtests/blenloader/CMakeLists.txt
+++ b/tests/gtests/blenloader/CMakeLists.txt
@@ -17,6 +17,34 @@
 # The Original Code is Copyright (C) 2019 by Blender Foundation.
 # ***** END GPL LICENSE BLOCK *****
 
+set(INC
+    .
+    ..
+    ../../../source/blender/blenkernel
+    ../../../source/blender/blenlib
+    ../../../source/blender/blenloader
+    ../../../source/blender/depsgraph
+    ../../../source/blender/imbuf
+    ../../../source/blender/makesdna
+    ../../../source/blender/makesrna
+    ../../../source/blender/windowmanager
+    ../../../intern/guardedalloc
+      ${GLOG_INCLUDE_DIRS}
+    ${GFLAGS_INCLUDE_DIRS}
+    ../../../extern/gtest/include
+)
+
+set(SRC
+  blendfile_loading_base_test.cc
+  blendfile_loading_base_test.h
+)
+
+set(LIB
+)
+
+blender_add_lib(bf_blenloader_test "${SRC}" "${INC}" "${INC_SYS}" "${LIB}")
+
+
 set(INC
     .
     ..
@@ -30,6 +58,7 @@ set(INC
 )
 
 set(LIB
+    bf_blenloader_test
     bf_blenloader
     bf_intern_opencolorio # Should not be needed but gives windows linker errors if the ocio libs are linked before this
     bf_gpu # Should not be needed but gives windows linker errors if the ocio libs are linked before this
@@ -50,7 +79,7 @@ BLENDER_SRC_GTEST_EX(
   NAME blenloader
   SRC "blendfile_load_test.cc;${_buildinfo_src}"
   EXTRA_LIBS "${LIB}"
-  EXTRA_CLI --test-assets-dir "${CMAKE_SOURCE_DIR}/../lib/tests")
+  COMMAND_ARGS --test-assets-dir "${CMAKE_SOURCE_DIR}/../lib/tests")
 
 unset(_buildinfo_src)
 
diff --git a/tests/gtests/blenloader/blendfile_load_test.cc b/tests/gtests/blenloader/blendfile_load_test.cc
index 37c7f604ba5..2ba3e3fcd88 100644
--- a/tests/gtests/blenloader/blendfile_load_test.cc
+++ b/tests/gtests/blenloader/blendfile_load_test.cc
@@ -15,15 +15,15 @@
  *
  * The Original Code is Copyright (C) 2019 by Blender Foundation.
  */
-#include "testing/blendfile_loading_test.h"
+#include "blendfile_loading_base_test.h"
 
-class BlendfileLoadingTest : public BlendfileLoadingAbstractTest {
+class BlendfileLoadingTest : public BlendfileLoadingBaseTest {
 };
 
 TEST_F(BlendfileLoadingTest, CanaryTest)
 {
   /* Load the smallest blend file we have in the SVN lib/tests directory. */
-  if (!blendfile_load("./modifier_stack/array_test.blend")) {
+  if (!blendfile_load("modifier_stack/array_test.blend")) {
     return;
   }
   depsgraph_create(DAG_EVAL_RENDER);
diff --git a/tests/gtests/testing/blendfile_loading_test.cc b/tests/gtests/blenloader/blendfile_loading_base_test.cc
similarity index 79%
rename from tests/gtests/testing/blendfile_loading_test.cc
rename to tests/gtests/blenloader/blendfile_loading_base_test.cc
index f5dccd2aa84..7c447e4fd77 100644
--- a/tests/gtests/testing/blendfile_loading_test.cc
+++ b/tests/gtests/blenloader/blendfile_loading_base_test.cc
@@ -15,7 +15,7 @@
  *
  * The Original Code is Copyright (C) 2019 by Blender Foundation.
  */
-#include "blendfile_loading_test.h"
+#include "blendfile_loading_base_test.h"
 
 extern "C" {
 #include "BKE_blender.h"
@@ -49,16 +49,16 @@ extern "C" {
 
 DEFINE_string(test_assets_dir, "", "lib/tests directory from SVN containing the test assets.");
 
-BlendfileLoadingAbstractTest::BlendfileLoadingAbstractTest()
+BlendfileLoadingBaseTest::BlendfileLoadingBaseTest()
     : testing::Test(), bfile(nullptr), depsgraph(nullptr)
 {
 }
 
-BlendfileLoadingAbstractTest::~BlendfileLoadingAbstractTest()
+BlendfileLoadingBaseTest::~BlendfileLoadingBaseTest()
 {
 }
 
-void BlendfileLoadingAbstractTest::SetUpTestCase()
+void BlendfileLoadingBaseTest::SetUpTestCase()
 {
   testing::Test::SetUpTestCase();
 
@@ -83,7 +83,7 @@ void BlendfileLoadingAbstractTest::SetUpTestCase()
   G.main->wm.first = MEM_callocN(sizeof(wmWindowManager), __func__);
 }
 
-void BlendfileLoadingAbstractTest::TearDown()
+void BlendfileLoadingBaseTest::TearDown()
 {
   depsgraph_free();
   blendfile_free();
@@ -91,28 +91,27 @@ void BlendfileLoadingAbstractTest::TearDown()
   testing::Test::TearDown();
 }
 
-bool BlendfileLoadingAbstractTest::blendfile_load(const char *filepath)
+bool BlendfileLoadingBaseTest::blendfile_load(const char *filepath)
 {
-  const char *test_assets_dir = FLAGS_test_assets_dir.c_str();
-  if (test_assets_dir == nullptr || test_assets_dir[0] == '\0') {
+  if (FLAGS_test_assets_dir.empty()) {
     ADD_FAILURE()
         << "Pass the flag --test-assets-dir and point to the lib/tests directory from SVN.";
     return false;
   }
 
   char abspath[FILENAME_MAX];
-  BLI_path_join(abspath, sizeof(abspath), test_assets_dir, filepath, NULL);
+  BLI_path_join(abspath, sizeof(abspath), FLAGS_test_assets_dir.c_str(), filepath, NULL);
 
   bfile = BLO_read_from_file(abspath, BLO_READ_SKIP_NONE, NULL /* reports */);
   if (bfile == nullptr) {
-    ADD_FAILURE();
+    ADD_FAILURE() << "Unable to load file '" << filepath << "' from test assets dir '"
+                  << FLAGS_test_assets_dir << "'";
     return false;
   }
-
   return true;
 }
 
-void BlendfileLoadingAbstractTest::blendfile_free()
+void BlendfileLoadingBaseTest::blendfile_free()
 {
   if (bfile == nullptr) {
     return;
@@ -121,7 +120,7 @@ void BlendfileLoadingAbstractTest::blendfile_free()
   bfile = nullptr;
 }
 
-void BlendfileLoadingAbstractTest::depsgraph_create(eEvaluationMode depsgraph_evaluation_mode)
+void BlendfileLoadingBaseTest::depsgraph_create(eEvaluationMode depsgraph_evaluation_mode)
 {
   depsgraph = DEG_graph_new(
       bfile->main, bfile->curscene, bfile->cur_view_layer, depsgraph_evaluation_mode);
@@ -129,7 +128,7 @@ void BlendfileLoadingAbstractTest::depsgraph_create(eEvaluationMode depsgraph_ev
   BKE_scene_graph_update_tagged(depsgraph, bfile->main);
 }
 
-void BlendfileLoadingAbstractTest::depsgraph_free()
+void BlendfileLoadingBaseTest::depsgraph_free()
 {
   if (depsgraph == nullptr) {
     return;
diff --git a/tests/gtests/testing/blendfile_loading_test.h b/tests/gtests/blenloader/blendfile_loading_base_test.h
similarity index 92%
rename from tests/gtests/testing/blendfile_loading_test.h
rename to tests/gtests/blenloader/blendfile_loading_base_test.h
index 8261717b8ce..4cbc0c4fef4 100644
--- a/tests/gtests/testing/blendfile_loading_test.h
+++ b/tests/gtests/blenloader/blendfile_loading_base_test.h
@@ -24,14 +24,14 @@
 struct BlendFileData;
 struct Depsgraph;
 
-class BlendfileLoadingAbstractTest : public testing::Test {
+class BlendfileLoadingBaseTest : public testing::Test {
  protected:
   struct BlendFileData *bfile;
   struct Depsgraph *depsgraph;
 
  public:
-  BlendfileLoadingAbstractTest();
-  virtual ~BlendfileLoadingAbstractTest();
+  BlendfileLoadingBaseTest();
+  virtual ~BlendfileLoadingBaseTest();
 
   /* Sets up Blender just enough to not crash on loading
    * a blendfile and constructing a depsgraph. */
@@ -52,7 +52,7 @@ class BlendfileLoadingAbstractTest : public testing::Test {
    * those will SEGFAULT.
    */
   bool blendfile_load(const char *filepath);
-  /* Free bfile if it is not nullptr */
+  /* Free bfile if it is not nullptr. */
   void blendfile_free();
 
   /* Create a depsgraph. Assumes a blend file has been loaded to this->bfile. */
diff --git a/tests/gtests/testing/CMakeLists.txt b/tests/gtests/testing/CMakeLists.txt
index 01081632cfa..f16c6223f19 100644
--- a/tests/gtests/testing/CMakeLists.txt
+++ b/tests/gtests/testing/CMakeLists.txt
@@ -20,15 +20,6 @@
 set(INC
   .
   ..
-  ../../../source/blender/blenkernel
-  ../../../source/blender/blenlib
-  ../../../source/blender/blenloader
-  ../../../source/blender/depsgraph
-  ../../../source/blender/imbuf
-  ../../../source/blender/makesdna
-  ../../../source/blender/makesrna
-  ../../../source/blender/windowmanager
-  ../../../intern/guardedalloc
   ${GLOG_INCLUDE_DIRS}
   ${GFLAGS_INCLUDE_DIRS}
   ../../../extern/gtest/include
@@ -38,10 +29,8 @@ set(INC_SYS
 )
 
 set(SRC
-  blendfile_loading_test.cc
   testing_main.cc
 
-  blendfile_loading_test.h
   testing.h
 )



More information about the Bf-blender-cvs mailing list