jsoncpp_readerwriter unit tests failed on Windows

[xkszltl]

Describe the bug

On windows, we always have 2 tests failed.

PS ********\jsoncpp\build> ctest
Test project ********/jsoncpp/build
    Start 1: jsoncpp_readerwriter
1/3 Test #1: jsoncpp_readerwriter ................***Failed    0.95 sec
    Start 2: jsoncpp_readerwriter_json_checker
2/3 Test #2: jsoncpp_readerwriter_json_checker ...***Failed    1.43 sec
    Start 3: jsoncpp_test
3/3 Test #3: jsoncpp_test ........................   Passed    0.01 sec

33% tests passed, 2 tests failed out of 3

Total Test time (real) =   2.41 sec

The following tests FAILED:
          1 - jsoncpp_readerwriter (Failed)
          2 - jsoncpp_readerwriter_json_checker (Failed)
Errors while running CTest

On Linux (CentOS 7/Ubuntu 18.04) they works fine.

Desktop (please complete the following information):

• OS: Windows Server 2019
• Ninja version 1.9.0
• CMake 3.17.2
• VS2019
• Python 3.8.2

[xkszltl]

Test log:

[cdunn2001]

You named the versions of everything except *jsoncpp itself. Why skip that one?

If this were a failure of a specific testcase, we would look into it right away. But the whole suite is broken? To debug that, we would need your help.

Also, we do not support cmake. Users do. I suggest trying meson. But if you have a fix for cmake, please submit a PR. We usually accept those pretty quickly In fact, I just merged one a few minutes ago, so perhaps the tip of master will help you. Maybe rebase your change at 58d552e onto that.

I was telling someone last week that it seems like half the Issues here relate to cmake. Drives me crazy! Anyway, please re-open if you can point to a more specific failure. Or submit a new PR, based on the current tip of `master.

[xkszltl]

@cdunn2001
jsoncpp version is the latest release tag, 1.9.2 currently.
I don't think this is cmake-related, because there's a post-build test at the end of build, which only runs jsoncpp_test and passed.
The 2 bad tests are triggered by a call to ctest afterward, so I'd like to know if they are deprecated.

[cdunn2001]

Oh, only legacy_test_array_0? are failing? Ok. That's unsettling. We always run via AppVeyor before merging.

There is currently a difference between the tests run via cmake and via meson. Fixed a race condition in meson. But I'll bet this is something else. We might need your help to debug it.

[xkszltl]

No just that. A lot of tests with legacy in name under jsoncpp_readerwriter_json_checker and jsoncpp_readerwriter, but not jsoncpp_test.
Let me know how I can help.

[xkszltl]

Ah never mind....I think I probably figure that out.
The dll isn't in the right place in build dir (probably on Linux we have rpath so everything's fine), and the test won't even start.
Once I symlink it to system32 everything's fine.

[xkszltl]

I can probably find some timeslot to help fixing the path in cmake.

[xkszltl]

[xkszltl]

jsoncpp/src/test_lib_json/CMakeLists.txt

Line 22 in 08ddeed

# First, copy the shared lib, for Microsoft.

This is why the other test can run.
No a good solution at all because on Windows usually cmake should write dll and exe to the same dir, but yes it works.

[jarrettchisholm]

Hello,

This change can end up causing an issue if you are using jsoncpp as a sub project in CMake.

Use of these variables was actually removed in commit 8371a43 in 2015 for this reason.

I understand that using the variables in this way fixes the issue with the test failure, but perhaps there is another way this can be fixed without using these variables?

[xkszltl]

Could you provide more details about the "issue"?
If you're talking about the output redirection, those var are CACHE so user should be able to overwrite them on-demand.

[xkszltl]

Another question will be: if parent project did something strange to these variables, should jsoncpp respect that and break, or should it ignore/overwrite that?

[jarrettchisholm]

The issue is basically if you include jsoncpp as a sub project, it will set these variables which end up getting used for any other projects part of the build.

You are right, I can override these variables, but then using jsoncpp ends up looking something like this:

add_subdirectory(jsoncpp)

set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}" CACHE PATH "Archive output dir." FORCE)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}" CACHE PATH "Library output dir." FORCE)
set(CMAKE_PDB_OUTPUT_DIRECTORY     "${CMAKE_BINARY_DIR}" CACHE PATH "PDB (MSVC debug symbol)output dir." FORCE)
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}" CACHE PATH "Executable/dll output dir." FORCE)

(I'm not sure if FORCE is necessary here.)

I think a good answer about this is here: https://stackoverflow.com/questions/44469983/proper-usage-of-cmake-output-directory

You could also try setting them on a project basis? Something like this:

set_target_properties( targets...
    PROPERTIES
    ARCHIVE_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/lib"
    LIBRARY_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/lib"
    RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
)

I haven't tested this, but maybe something like that could work for you and not interfere with anything trying to use jsoncpp as a sub project?

[xkszltl]

I think the issue is, those variables are not suppose to be visible to parent scope.
Maybe CACHE is the problem.

Setting per-target properties is a good way, but I'm not familiar with details in jsoncpp, so should be better for its author to do the work.
The down side is, we lost the centralized place for management.

Another solution is, to detect what was set before jsoncpp makes changes, and restore them afterward.