Page MenuHome

[Cycles/MSVC/Testing] Fix broken test code.

Authored by LazyDodo (LazyDodo) on Nov 18 2016, 10:00 PM.



Currently the tests don't run on windows for the following reasons

  1. render_graph_finalize has an linking issue due missing a bunch of libraries (not sure why this is not an issue for linux)
  2. This one is more interesting, in test/python/cmakelists.txt ${TEST_BLENDER_EXE_BARE} and ${TEST_BLENDER_EXE} are flat out wrong, but for some reason this doesn't matter for most tests, cause ctest will actually go out and look for the executable and fix the path for you *BUT* only for the command, if you use them in any of the parameters it'll happily pass on the wrong path.
  3. on linux you can just run a .py file, windows is not as awesome and needs to be told to run it with pyton.
  4. had to use the NAME/COMMAND long form of add_test otherwise $<TARGET_FILE:blender> doesn't get expanded, why? beats me.
  5. missing idiff.exe for msvc2015/x64 in the libs folder.

This patch addresses 1-4 , but given I have no working Linux build environment, I'm unsure if it'll break anything there

5 has been fixed in rBL61751

Diff Detail

rB Blender

Event Timeline

LazyDodo (LazyDodo) retitled this revision from to [Cycles/MSVC/Testing] Fix broken test code..
LazyDodo (LazyDodo) updated this object.
LazyDodo (LazyDodo) set the repository for this revision to rB Blender.
LazyDodo (LazyDodo) added projects: Cycles, Restricted Project.

Is that really needed?

AFAIK, we wanted to make it so all FOO_LIBRARY are absolute paths.


Not sure why this is only needed for Cycles tests. There are other tests which are using TEST_BLENDER_EXE. So mu guess is that they will also fail and that we should have something done at the beginning of the file where we calculate TEST_BLENDER_EXE.


Copied this from the creator cmakelists.txt, atleast for windows most paths are absolute, but some of them are just the libname. (libpng/openimageio/boost) png and openimageio i can probably convert to full paths, boost given it's depending on the system findboost.cmake seems unlikely. so we'll still need atleast that.


See point 2, the cycles tests are the only ones that use the TEST_BLENDER_EXE_BARE as a parameter, if it's the command, ctest sighs and fixes the path silently for you, but not for parameters.



Well, guess we'll worry about converting libs to abspath later.


Wouldn't it make sense to set TEST_BLENDER_EXE to "$<TARGET_FILE:blender>" so we don't care how we use the command?


I tried, but cmake refuses to expand it, I had to use $<TARGET_FILE:blender> if i used


            COMMAND python ${CMAKE_CURRENT_LIST_DIR}/
            -blender "${TEST_BLENDER_EXE_BARE}"
            -testdir "${TEST_SRC_DIR}/cycles/ctests/render"
            -idiff "${OPENIMAGEIO_IDIFF}"

it resulted in the following command it tried to execute

20: Test command: python.exe "K:/BlenderGit/blender/tests/python/" "-blender" "$<TARGET_FILE:blender>" "-testdir" "K:/BlenderGit/blender/../lib/tests/cycles/ctests/re
nder" "-idiff" "K:/BlenderGit/blender/../lib/win64_vc14/openimageio/bin/idiff.exe"

which naturally failed badly.

Sergey Sharybin (sergey) edited edge metadata.
Sergey Sharybin (sergey) added inline comments.

Weirdly it worked in cmake-3.6.2 but not in cmake-3.7.1 for me.

Well, guess better to solve the issue for now and dig into cmake specifics later.

This revision is now accepted and ready to land.Dec 8 2016, 3:50 PM
LazyDodo (LazyDodo) edited edge metadata.
LazyDodo (LazyDodo) removed rB Blender as the repository for this revision.

While running sanity checks before committing this diff, more problems showed up

  1. When building with msvc2015/BLI_string_test , EXPECT_EQ_ARRAY_ND can't pick the right template between int and const int , causing a build error.
  2. Soo many linker errors when using msvc2015,with intern/cycles/test/CMakeLists.txt , at a closer look intern/cycles/test/CMakeLists.txt is redoing pretty much all the work setup_liblinks was already doing (but poorly, was linking in some release libs into debug builds causing chaos), no need to have the same code in 2 places. removed it.
  3. OpenSubDiv weirdness in setup_liblinks , it tries to be clever and figure out the debug libs, not sure why we handled this this way, all other libs are managed in platform_win32_msvc.cmake fixed the problem there and removed the exception in setup_liblinks.
  4. Some tests require additional shared libraries (bmesh_core_test for example, but there's others) those failed, because they simply couldn't start, changed the working directory for tests to be the blender output folder, where those dll's wil be guaranteed to be found.
  5. Windows doesn't have python build in or as it is on my local system python2 is found in the path, our script have some python 3 code in them. counting on python3 in the path is a bit annoying, we know where we can find a python3 for sure(2.78/python/bin) since we ship it ourselves.
  6. CTestTesting.cmake was lacking ${GFLAGS_DEFINES} ${GLOG_DEFINES} in causing linker errors with msvc2015

I validated all these changes against msvc 2013/2015 x64 all tests run now(good), but 4 fail (bad, but this diff is about getting the tests working, not passing)

I know this diff has been accepted already , but given i've done some rather big changes I'd like a sober second look (and perhaps a linux user to take a quick peek and check if haven't broken anything)

Sergey Sharybin (sergey) requested changes to this revision.Dec 12 2016, 9:52 AM
Sergey Sharybin (sergey) edited edge metadata.
Sergey Sharybin (sergey) added inline comments.

This doesn't seems a good place to have this. BLENDER_SRC_GTEST_EX is a macro, meaning all modifications ot the environment are propagated to the parent scope. This means, that if you have multiple subsequent calls to this macro you'll be adding GFlags/GLog defines again and again.

My guess is that you should:

  • Revert this chunk of change
  • Remove add_definitions() from tests/gtests/testing/CMakeLists.txt
  • Add this defines to tests/gtests/CMakeLists.txt

You can't do this from within Cycles directory since it should be kept snadalone-alike. There is no such macro in Cycles repository and and wouldn't want to being WHOLE blender-specific libraries linking thingie to a standalone Cycles repo.


Same as above.


I don't luck such a compiler idiocy and don't like discarding const qualifier just to make something compiled.

Would that work if we make it const T *actual? It's not like we are modifying it in the EXPECT function anyway.

This revision now requires changes to proceed.Dec 12 2016, 9:52 AM
LazyDodo (LazyDodo) edited edge metadata.

I'm open for an alternative for the monstrosity that is the python path, it's the only way i found that supported both the release and debug version on windows, while also not breaking on linux.

Sergey Sharybin (sergey) requested changes to this revision.Jan 25 2017, 2:44 PM
Sergey Sharybin (sergey) edited edge metadata.
Sergey Sharybin (sergey) added inline comments.

The new code is wrong here for linux. It's no t guaranteed to have /use/bin/python to point to a python3 install. Can we leave it up to shebang to decide which python to use? Alternatively, we should make it /usr/bin/python3.

This revision now requires changes to proceed.Jan 25 2017, 2:44 PM

for linux it's just calling 'python' with no path like the original script did. the only bit that applies for linux is


(if MSVC not defined->python)

the rest of this abomination falls in the MSVC defined clause


Original CMake did not have any explicit mention of python and simply called ${CMAKE_CURRENT_LIST_DIR}/, leaving actual intepreter to be deducted from shebang. Now the CMake calls python which gets expanded to /usr/bin/python which is not necessarily a python3.


Ah yikes, the only way to restore the old functionality would be something like


but this is getting ridiculous , i think i'm gonna have another stab at using the blender.exe as the python interpreter

LazyDodo (LazyDodo) edited edge metadata.
LazyDodo (LazyDodo) set the repository for this revision to rB Blender.

Updated with your macro, much nicer.

This revision is now accepted and ready to land.Jan 25 2017, 5:29 PM
This revision was automatically updated to reflect the committed changes.