Page MenuHome

Blendfile-loading test class
ClosedPublic

Authored by Sybren A. Stüvel (sybren) on Wed, Nov 13, 4:42 PM.

Details

Summary

To test some functionality of the USD exporter, I want to be able to load a blend file and run parts of the exporter. Up to now this wasn't possible from a GTest test. Of course a Python test could run the USD exporter, but testing the result would require writing a parser for USD files, which I want to avoid.

This new test class minimally sets up Blender so that it can load blend files and construct a depsgraph without crashing. Note that I haven't tested this on very complex blend files, so it may still crash when the loaded blend file references/requires uninitialised data structures.

The test will certainly crash with Blend files created with Blender 2.80, as the versioning code requires space types to be registered. This is normally done by initialising the window manager, which is not done in this test. The WM requires Python to run, which in turn requires that Blender finds the release directory in the same directory that contains the running executable, which is not the case for GTest tests (they are written to bin/tests/executablename.

Diff Detail

Repository
rB Blender

Event Timeline

  • Added GPL header to each new file

Rest LGTM. As this is a test utility we can always adapt.

tests/gtests/blenloader/CMakeLists.txt
18

2019

tests/gtests/blenloader/blendfile_load_test.cc
17

Time machine :-)

tests/gtests/testing/blendfile_loading_test.cc
16 ↗(On Diff #19602)

Same as above

tests/gtests/testing/blendfile_loading_test.h
16 ↗(On Diff #19602)

Same as above.

  • Updated license blocks
Sergey Sharybin (sergey) requested changes to this revision.Thu, Nov 14, 3:53 PM
Sergey Sharybin (sergey) added inline comments.
build_files/cmake/Modules/GTestTesting.cmake
17–22

What is the purpose of this comment?
If one is not familiar with CMake he/she can type cmake cmake_parse_arguments in the search engine of choice and have the page linked in the first few results.

We can not start adding links to every function in CMake, Cocoa, Win32 API documentation.

20

While EXTRA_LIBS kind of truly implies that there will be more libraries added (as in, default ones will be specified, and then the user specified ones), same logic is wrong and confusing for EXTRA_CLI. You are not adding extra arguments, but you are providing them.

P.S. Not sure i feel about CLI in the name. It's not a CLI what you're providing, but command line arguments. CLA? Hrm..

84–104

With the keyword based easy-to-use-and-follow BLENDER_SRC_GTEST_EX do we really need all of those wrappers?

tests/gtests/blenloader/blendfile_load_test.cc
21

Abstract in the name of a class implies it is an abstract class which can not be instantiated and is expected to be sub-classed, filling in some missing bits.

Suggest to use BlendfileLoadingBaseTest.

tests/gtests/testing/CMakeLists.txt
23–44 ↗(On Diff #19624)

I am not happy with all of this:

  • This transparently allows to include files from many places in any non-blenderfile-loading test.
  • This adds blend-file-loading symbols (and dependencies) to tests which don't need this at all.
tests/gtests/testing/blendfile_loading_test.cc
61 ↗(On Diff #19624)

By setting up an environment but never freeing it you're risking having real memory leaks.
Ideally we should fail any test when leak is detected.

Should be possible to initialize subsystems in SetUp() and de-initialize them in TearDown().

65–76 ↗(On Diff #19624)

This is quite fragile. Any way to de-duplicate it to have only in a single place?

96–97 ↗(On Diff #19624)

c_str()returns a pointer to a null-terminated string. So check test_assets_dir == nullptr is redundant and confusing for someone familiar with C++.

For the readability use foo.empty().

108 ↗(On Diff #19624)

Missing message.

tests/gtests/testing/blendfile_loading_test.h
22 ↗(On Diff #19624)

In blender it's more a convention of " for own headers and <> for system ones.

Also not sure why you need it here. Use forward declarations in headers unless absolutely needed to include.

55 ↗(On Diff #19624)

Fullstop.

This revision now requires changes to proceed.Thu, Nov 14, 3:53 PM
Sybren A. Stüvel (sybren) marked 6 inline comments as done.Thu, Nov 14, 4:42 PM
Sybren A. Stüvel (sybren) added inline comments.
build_files/cmake/Modules/GTestTesting.cmake
17–22

Damn, when updating the patch Arcanist didn't remember the 'base' argument I included earlier, and also included the changes in D6236. We can just do the review here, and I'll commit in separate commits (one for the keyword arguments of BLENDER_SRC_GTEST_EX and one for the blendfile loading test class).

20

COMMAND_ARGS seems appropriate to me. It's arguments for the thing that's passed as COMMAND to add_test().

84–104

It would turn a list like this:

BLENDER_TEST(BLI_array "bf_blenlib")
BLENDER_TEST(BLI_array_ref "bf_blenlib")
BLENDER_TEST(BLI_array_store "bf_blenlib")
BLENDER_TEST(BLI_array_utils "bf_blenlib")
BLENDER_TEST(BLI_delaunay_2d "bf_blenlib")
BLENDER_TEST(BLI_edgehash "bf_blenlib")
BLENDER_TEST(BLI_expr_pylike_eval "bf_blenlib")
BLENDER_TEST(BLI_ghash "bf_blenlib")
BLENDER_TEST(BLI_hash_mm2a "bf_blenlib")
BLENDER_TEST(BLI_heap "bf_blenlib")
BLENDER_TEST(BLI_heap_simple "bf_blenlib")
BLENDER_TEST(BLI_index_range "bf_blenlib")
BLENDER_TEST(BLI_kdopbvh "bf_blenlib;bf_intern_numaapi")
BLENDER_TEST(BLI_linklist_lockfree "bf_blenlib;bf_intern_numaapi")
BLENDER_TEST(BLI_listbase "bf_blenlib")
BLENDER_TEST(BLI_map "bf_blenlib")
BLENDER_TEST(BLI_math_base "bf_blenlib")
BLENDER_TEST(BLI_math_color "bf_blenlib")
BLENDER_TEST(BLI_math_geom "bf_blenlib")
BLENDER_TEST(BLI_memiter "bf_blenlib")
BLENDER_TEST(BLI_path_util "${BLI_path_util_extra_libs}")
BLENDER_TEST(BLI_polyfill_2d "bf_blenlib")
BLENDER_TEST(BLI_set "bf_blenlib")
BLENDER_TEST(BLI_stack "bf_blenlib")
BLENDER_TEST(BLI_stack_cxx "bf_blenlib")
BLENDER_TEST(BLI_string "bf_blenlib")
BLENDER_TEST(BLI_string_map "bf_blenlib")
BLENDER_TEST(BLI_string_ref "bf_blenlib")
BLENDER_TEST(BLI_string_utf8 "bf_blenlib")
BLENDER_TEST(BLI_task "bf_blenlib;bf_intern_numaapi")
BLENDER_TEST(BLI_vector "bf_blenlib")
BLENDER_TEST(BLI_vector_set "bf_blenlib")

into a list like this:

BLENDER_SRC_GTEST_EX(NAME BLI_array SRC BLI_array_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_array_ref SRC BLI_array_ref_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_array_store SRC BLI_array_store_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_array_utils SRC BLI_array_utils_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_delaunay_2d SRC BLI_delaunay_2d_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_edgehash SRC BLI_edgehash_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_expr_pylike_eval SRC BLI_expr_pylike_eval_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_ghash SRC BLI_ghash_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_hash_mm2a SRC BLI_hash_mm2a_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_heap SRC BLI_heap_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_heap_simple SRC BLI_heap_simple_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_index_range SRC BLI_index_range_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_kdopbvh SRC BLI_kdopbvh_test.cc EXTRA_LIBS "bf_blenlib;bf_intern_numaapi")
BLENDER_SRC_GTEST_EX(NAME BLI_linklist_lockfree SRC BLI_linklist_lockfree_test.cc EXTRA_LIBS "bf_blenlib;bf_intern_numaapi")
BLENDER_SRC_GTEST_EX(NAME BLI_listbase SRC BLI_listbase_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_map SRC BLI_map_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_math_base SRC BLI_math_base_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_math_color SRC BLI_math_color_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_math_geom SRC BLI_math_geom_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_memiter SRC BLI_memiter_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_path_util SRC BLI_path_util_test.cc EXTRA_LIBS "${BLI_path_util_extra_libs}")
BLENDER_SRC_GTEST_EX(NAME BLI_polyfill_2d SRC BLI_polyfill_2d_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_set SRC BLI_set_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_stack SRC BLI_stack_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_stack_cxx SRC BLI_stack_cxx_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_string SRC BLI_string_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_string_map SRC BLI_string_map_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_string_ref SRC BLI_string_ref_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_string_utf8 SRC BLI_string_utf8_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_task SRC BLI_task_test.cc EXTRA_LIBS "bf_blenlib;bf_intern_numaapi")
BLENDER_SRC_GTEST_EX(NAME BLI_vector SRC BLI_vector_test.cc EXTRA_LIBS "bf_blenlib")
BLENDER_SRC_GTEST_EX(NAME BLI_vector_set SRC BLI_vector_set_test.cc EXTRA_LIBS "bf_blenlib")

I'd say it's worth it to keep those aliases.

tests/gtests/testing/CMakeLists.txt
23–44 ↗(On Diff #19624)

I see two solutions

  1. Put the BlendfileLoadingBaseTest code in the tests/gtests/blenloader module as well, and have other test modules link to that if necessary.
  2. Create a new tests/gtests/blenloader_base_test module.

I think I'd prefer the first option, do you have any preference @Sergey Sharybin (sergey)?

tests/gtests/testing/blendfile_loading_test.cc
61 ↗(On Diff #19624)

I considered that, but leak detection is already turned off for tests because external things (like Python itself) already don't cleanly deallocate on exit.

65–76 ↗(On Diff #19624)

I agree, but deduplication might be tricky as well. The startup cannot be done fully, because that would imply starting the window manager, which tries to run Python code from the 'release' directory, which doesn't exist in bin/tests. So, because the startup couldn't be done fully anyway, I cherry-picked those parts of the initialisation that are required for loading a simple blend file.

108 ↗(On Diff #19624)

A message is already printed by BLO_read_from_file(), but I can add another one here.

tests/gtests/testing/blendfile_loading_test.h
22 ↗(On Diff #19624)

It's necessary for void depsgraph_create(eEvaluationMode depsgraph_evaluation_mode);, since you can't forward-declare enums.

Sybren A. Stüvel (sybren) marked 3 inline comments as done.Thu, Nov 14, 4:45 PM

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.
build_files/cmake/Modules/GTestTesting.cmake
84–104

Nice verbose. Never mind then.

tests/gtests/blenloader/CMakeLists.txt
33

Indentation?

tests/gtests/blenloader/blendfile_loading_base_test.cc
54

Thin king out loud: shall we promote in-class member initialization?

Downside is that changing default will re-compile all files which include the header.

Upside is that we will never forget to initialize bhead and depsgraph to nullptr.

tests/gtests/blenloader/blendfile_loading_base_test.h
19

__BLENDFILE_LOADING_BASE_TEST_H__

tests/gtests/testing/CMakeLists.txt
17 ↗(On Diff #19628)

We don't really bump year in an existing files.
We could do that, but outside of the actual logic change.

Sybren A. Stüvel (sybren) marked an inline comment as done.Fri, Nov 15, 2:12 PM
Sybren A. Stüvel (sybren) added inline comments.
tests/gtests/blenloader/blendfile_loading_base_test.cc
54

In-class member initialisation looks good, yes.

  • More feedback from Sergey
Sybren A. Stüvel (sybren) marked 5 inline comments as done.
  • Header guard #define
Sybren A. Stüvel (sybren) marked 5 inline comments as done.Fri, Nov 15, 2:27 PM
  • More feedback from Sergey
  • Deinitialisation of Blender

@Bastien Montagne (mont29) @Campbell Barton (campbellbarton) loading a blendfile & freeing it again seems to cause a memory leak, which neither @Sergey Sharybin (sergey) nor me could figure out. Can you offer us any insight?

sybren@ws-sybren ~/workspace/blender-git/build_debug 
% ninja ./bin/tests/blenloader_test && ./bin/tests/blenloader_test --test-assets-dir /home/sybren/workspace/blender-git/build_debug/../lib/tests
[3/3] Linking CXX executable bin/tests/blenloader_test
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from BlendfileLoadingTest
Color management: using fallback mode for management
Color management: Error could not find role data role.
[ RUN      ] BlendfileLoadingTest.CanaryTest
[       OK ] BlendfileLoadingTest.CanaryTest (31 ms)
Error: Not freed memory blocks: 3, total unfreed memory 0.000717 MB
[----------] 1 test from BlendfileLoadingTest (31 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (49 ms total)
[  PASSED  ] 1 test.

=================================================================
==16434==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 344 byte(s) in 1 object(s) allocated from:
    #0 0x7fa291114448 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10c448)
    #1 0x5636b898db9d in MEM_lockfree_mallocN /home/sybren/workspace/blender-git/blender/intern/guardedalloc/intern/mallocn_lockfree_impl.c:308
    #2 0x5636b87d951c in read_struct /home/sybren/workspace/blender-git/blender/source/blender/blenloader/intern/readfile.c:2258
    #3 0x5636b884fd10 in read_data_into_oldnewmap /home/sybren/workspace/blender-git/blender/source/blender/blenloader/intern/readfile.c:9124
    #4 0x5636b88513d8 in read_libblock /home/sybren/workspace/blender-git/blender/source/blender/blenloader/intern/readfile.c:9280
    #5 0x5636b8857517 in blo_read_file_internal /home/sybren/workspace/blender-git/blender/source/blender/blenloader/intern/readfile.c:9847
    #6 0x5636b87bff78 in BLO_read_from_file /home/sybren/workspace/blender-git/blender/source/blender/blenloader/intern/readblenentry.c:320
    #7 0x5636b87ba07c in BlendfileLoadingBaseTest::blendfile_load(char const*) /home/sybren/workspace/blender-git/blender/tests/gtests/blenloader/blendfile_loading_base_test.cc:134
    #8 0x5636b87b4515 in BlendfileLoadingTest_CanaryTest_Test::TestBody() /home/sybren/workspace/blender-git/blender/tests/gtests/blenloader/blendfile_load_test.cc:26
    #9 0x5636b8a16040 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2402
    #10 0x5636b8a0174d in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2438
    #11 0x5636b89ac903 in testing::Test::Run() /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2474
    #12 0x5636b89af790 in testing::TestInfo::Run() /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2656
    #13 0x5636b89b2105 in testing::TestCase::Run() /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2774
    #14 0x5636b89d6cdb in testing::internal::UnitTestImpl::RunAllTests() /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:4649
    #15 0x5636b8a1b194 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2402
    #16 0x5636b8a05aff in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2438
    #17 0x5636b89cc624 in testing::UnitTest::Run() /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:4257
    #18 0x5636b89862f5 in RUN_ALL_TESTS() /home/sybren/workspace/blender-git/blender/extern/gtest/include/gtest/gtest.h:2233
    #19 0x5636b89860fc in main /home/sybren/workspace/blender-git/blender/tests/gtests/testing/testing_main.cc:28
    #20 0x7fa28ccbbb6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)

Indirect leak of 56 byte(s) in 2 object(s) allocated from:
    #0 0x7fa291114448 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10c448)
    #1 0x5636b898db9d in MEM_lockfree_mallocN /home/sybren/workspace/blender-git/blender/intern/guardedalloc/intern/mallocn_lockfree_impl.c:308
    #2 0x5636b87d951c in read_struct /home/sybren/workspace/blender-git/blender/source/blender/blenloader/intern/readfile.c:2258
    #3 0x5636b884fd10 in read_data_into_oldnewmap /home/sybren/workspace/blender-git/blender/source/blender/blenloader/intern/readfile.c:9124
    #4 0x5636b88513d8 in read_libblock /home/sybren/workspace/blender-git/blender/source/blender/blenloader/intern/readfile.c:9280
    #5 0x5636b8857517 in blo_read_file_internal /home/sybren/workspace/blender-git/blender/source/blender/blenloader/intern/readfile.c:9847
    #6 0x5636b87bff78 in BLO_read_from_file /home/sybren/workspace/blender-git/blender/source/blender/blenloader/intern/readblenentry.c:320
    #7 0x5636b87ba07c in BlendfileLoadingBaseTest::blendfile_load(char const*) /home/sybren/workspace/blender-git/blender/tests/gtests/blenloader/blendfile_loading_base_test.cc:134
    #8 0x5636b87b4515 in BlendfileLoadingTest_CanaryTest_Test::TestBody() /home/sybren/workspace/blender-git/blender/tests/gtests/blenloader/blendfile_load_test.cc:26
    #9 0x5636b8a16040 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2402
    #10 0x5636b8a0174d in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2438
    #11 0x5636b89ac903 in testing::Test::Run() /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2474
    #12 0x5636b89af790 in testing::TestInfo::Run() /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2656
    #13 0x5636b89b2105 in testing::TestCase::Run() /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2774
    #14 0x5636b89d6cdb in testing::internal::UnitTestImpl::RunAllTests() /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:4649
    #15 0x5636b8a1b194 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2402
    #16 0x5636b8a05aff in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:2438
    #17 0x5636b89cc624 in testing::UnitTest::Run() /home/sybren/workspace/blender-git/blender/extern/gtest/src/gtest.cc:4257
    #18 0x5636b89862f5 in RUN_ALL_TESTS() /home/sybren/workspace/blender-git/blender/extern/gtest/include/gtest/gtest.h:2233
    #19 0x5636b89860fc in main /home/sybren/workspace/blender-git/blender/tests/gtests/testing/testing_main.cc:28
    #20 0x7fa28ccbbb6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)

SUMMARY: AddressSanitizer: 400 byte(s) leaked in 3 allocation(s).

The problem is the windows aren't being freed, this requires WM_init to set the wm_close_and_free callback which is called to free window data.

The problem is the windows aren't being freed, this requires WM_init to set the wm_close_and_free callback which is called to free window data.

Thanks for the pointer, I called wm_close_and_free() and the memory leak is gone.

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)
  • Fixed memory leak
  • Merge remote-tracking branch 'origin/master' into wip-ctest-blenloader

@Sergey Sharybin (sergey) @Bastien Montagne (mont29) I've handled the feedback so far. Anything else that's needed to get this accepted?

Picky thing about duplicated comment. Otherwise LGTM.

tests/gtests/blenloader/CMakeLists.txt
64–65
set(LIB
    bf_blenloader_test
    bf_blenloader

 Should not be needed but gives windows linker errors if the OCIO libs are linked before this.
  bf_intern_opencolorio
  bf_gpu
)
This revision is now accepted and ready to land.Thu, Nov 28, 11:33 AM
  • Deduplicated comments
  • Nicer structure of the CMakeLists.txt that makes it easier to extend the test with multiple source files. I needed this in the USD tests, and adding to a semicolon-separated string isn't as nice.

LGTM from a quick check. :)

tests/gtests/blenloader/blendfile_loading_base_test.cc
80

ot -> to find

This revision was automatically updated to reflect the committed changes.