3.8.0: unit-cbor.cpp test failures

[dvzrv]

Hi! I package nlohmann-json for Arch Linux. Upon building version 3.8.0 I ran the unit tests, but some fail.

For the last stable release (3.7.3) I used the below described workflow and the unit tests pass. Maybe something has changed in the meantime?

What is the issue you have?

Executing test-algorithms...
[doctest] doctest version is "2.3.7"
[doctest] run with "--help" for options
===============================================================================
[doctest] test cases:      1 |      1 passed |      0 failed |      0 skipped
[doctest] assertions:     39 |     39 passed |      0 failed |
[doctest] Status: SUCCESS!
Executing test-allocator...
[doctest] doctest version is "2.3.7"
[doctest] run with "--help" for options
===============================================================================
[doctest] test cases:      3 |      3 passed |      0 failed |      0 skipped
[doctest] assertions:     17 |     17 passed |      0 failed |
[doctest] Status: SUCCESS!
Executing test-alt-string...
[doctest] doctest version is "2.3.7"
[doctest] run with "--help" for options
===============================================================================
[doctest] test cases:      1 |      1 passed |      0 failed |      0 skipped
[doctest] assertions:     28 |     28 passed |      0 failed |
[doctest] Status: SUCCESS!
Executing test-bson...
[doctest] doctest version is "2.3.7"
[doctest] run with "--help" for options
===============================================================================
[doctest] test cases:      6 |      6 passed |      0 failed |      1 skipped
[doctest] assertions:    486 |    486 passed |      0 failed |
[doctest] Status: SUCCESS!
Executing test-capacity...
[doctest] doctest version is "2.3.7"
[doctest] run with "--help" for options
===============================================================================
[doctest] test cases:      1 |      1 passed |      0 failed |      0 skipped
[doctest] assertions:    120 |    120 passed |      0 failed |
[doctest] Status: SUCCESS!
Executing test-cbor...
[doctest] doctest version is "2.3.7"
[doctest] run with "--help" for options
===============================================================================
src/unit-cbor.cpp:1912:
TEST CASE:  single CBOR roundtrip
  sample.json

src/unit-cbor.cpp:1912: ERROR: test case THREW exception: exception thrown in subcase - will translate later when the whole test case has been exited (cannot translate while there is an active exception)

===============================================================================
src/unit-cbor.cpp:1912:
TEST CASE:  single CBOR roundtrip

src/unit-cbor.cpp:1912: ERROR: test case THREW exception: [json.exception.parse_error.101] parse error at line 1, column 1: syntax error while parsing value - unexpected end of input; expected '[', '{', or a literal

===============================================================================
src/unit-cbor.cpp:2350:
TEST CASE:  examples from RFC 7049 Appendix A
  byte arrays

src/unit-cbor.cpp:2500: ERROR: CHECK_NOTHROW( j = json::from_cbor(packed) ) THREW exception: "[json.exception.parse_error.110] parse error at byte 1: syntax error while parsing CBOR value: unexpected end of input"

src/unit-cbor.cpp:2505: ERROR: CHECK( j == json::binary(expected) ) is NOT correct!
  values: CHECK( null == {"bytes":[],"subtype":null} )

===============================================================================
[doctest] test cases:      5 |      3 passed |      2 failed |      1 skipped
[doctest] assertions: 1640832 | 1640830 passed |      2 failed |
[doctest] Status: FAILURE!

Full build log:
nlohmann-json-3.8.0-build.log

Please describe the steps to reproduce the issue.

I'm cloning this repository from the commit (as no annotated tags exist) and verify the lightweight tag/commit using the author's PGP signature.

I'm building with:

  cmake -DCMAKE_INSTALL_PREFIX='/usr' \
        -DCMAKE_INSTALL_LIBDIR='/usr/lib' \
        -DCMAKE_BUILD_TYPE='None' \
        -DBUILD_TESTING=ON \
        -DJSON_MultipleHeaders=ON \
        -Wno-dev \
        -B build \
        -S .
  make VERBOSE=1 -C build

Additional to the above I run the following to get pkgconfig integration (but that has nothing to do with the tests):

  meson --prefix=/usr \
        --libdir=lib \
        meson-build
  ninja -C meson-build

I run tests with:

make -k check

Note:
When running the test suite using ctest (e.g. as in use in the travis integration)

json/.travis.yml

Lines 324 to 328 in e7452d8

	# compile and execute unit tests
	- mkdir -p build && cd build
	- cmake .. ${CMAKE_OPTIONS} -DJSON_MultipleHeaders=${MULTIPLE_HEADERS} -DJSON_BuildTests=On -GNinja && cmake --build . --config Release
	- ctest -C Release --timeout 2700 -V -j
	- cd ..

No tests are collected:

UpdateCTestConfiguration  from :/build/nlohmann-json/src/nlohmann-json-3.8.0/DartConfiguration.tcl
UpdateCTestConfiguration  from :/build/nlohmann-json/src/nlohmann-json-3.8.0/DartConfiguration.tcl
Test project /build/nlohmann-json/src/nlohmann-json-3.8.0
Constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
No tests were found!!!

Can you provide a small but working code example?

n/a

What is the expected behavior?

All unit-tests pass

And what is the actual behavior instead?

Some unit-cbor unit tests fail.

Which compiler and operating system are you using?

• Compiler: gcc 10.1.0
• Operating system: Arch Linux

Which version of the library did you use?

• latest release version 3.7.3
• other release - please state the version: 3.8.0
• the develop branch

If you experience a compilation error: can you compile and run the unit tests?

• yes
• no - please copy/paste the error message below

see above

[nlohmann]

make check is no longer supported. I need to update the Makefile accordingly to avoid such confusion.

The test suite is now in a separate repository and is downloaded as part of ctest.

Can you please try to execute the unit tests as described in https://github.com/nlohmann/json#execute-unit-tests:

mkdir build
cd build
cmake .. -DJSON_BuildTests=On
cmake --build .
ctest --output-on-failure

[dvzrv]

@nlohmann Thanks for getting back on this so quickly.

I see. Hm, I think downloading during build/test is not possible though. Can I download the tarball of the tests myself and extract it to a directory where it will be picked up?
This way I can have validated sources upon download :)

[nlohmann]

The code that does the download is here: https://github.com/nlohmann/json/blob/develop/cmake/download_test_data.cmake#L7, so this is possible:

mkdir build
cd build
cmake ..
make download_test_data

The test files are then downloaded to json_test_data inside build.

[dvzrv]

Okay, I can get it to work by downloading the required test data and then:

  mkdir -vp build
  mv -v json_test_data-2.0.0 build/json_test_data  
  cmake -DCMAKE_INSTALL_PREFIX='/usr' \
        -DCMAKE_INSTALL_LIBDIR='/usr/lib' \
        -DCMAKE_BUILD_TYPE='None' \
        -DBUILD_TESTING=ON \
        -DJSON_MultipleHeaders=ON \
        -Wno-dev \
        -B build \
        -S .
  make VERBOSE=1 -C build
  cd build
  ctest  --output-on-failure

I think it would be really really awesome to have a cmake option to point at a directory with the test data. For packagers this would be the preferred way, because it allows for properly doing checksums on tarballs for the test data, extract it and run the setup.
Basically making this here more flexible:

json/cmake/download_test_data.cmake

Lines 7 to 14 in e7452d8

	add_custom_target(download_test_data
	COMMAND test -d json_test_data || ${GIT_EXECUTABLE} clone -c advice.detachedHead=false --branch v${JSON_TEST_DATA_VERSION} ${JSON_TEST_DATA_URL}.git --quiet --depth 1
	COMMENT "Downloading test data from ${JSON_TEST_DATA_URL} (v${JSON_TEST_DATA_VERSION})"
	WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
	)
	# create a header with the path to the downloaded test data
	file(WRITE ${CMAKE_BINARY_DIR}/include/test_data.hpp "#define TEST_DATA_DIRECTORY \"${CMAKE_BINARY_DIR}/json_test_data\"\n")

[nlohmann]

Sorry, I do not understand your use case. Why do you want to checksum the test data? Why isn't the current approach sufficient?

[dvzrv]

Because this would download assets during build time.

On the background:
Packaging in Arch's case is separated into a preparation, build, test and package step. The cmake and make calls traditionally happen during build, the ctest call during the test step.
All assets required to build a piece of software should be locked by either commit/tag + signature or tarball + checksum and to be known prior to prepare/build/test/package to ensure a reproducible build.
"Exceptions" to this rule are packages relying on language specific package management (e.g. nodejs, golang or rust, which we link statically, but which lock their own dependencies by either commit hash or version). Partially this is of course also because packaging e.g. each one-liner nodejs library requires many many developers.

We have a policy (and this is quite similar to that of other distributions which even enforce this by disabling network services in their build infrastructure during build) to not download assets in the build or test steps (and certainly not in the package step of course), as these are files that we can not account for and potentially break a reproducible build.
Distributions such as Debian (with many more developers than Arch has) go to great lengths to patch these kind of scenarios and for many upstreams even have an entire copy of the source code just in case or have started packaging many hundreds of language-specific source packages (e.g. golang, java, nodejs) to build packages with packaged sources, etc.

[nlohmann]

I understand. I shall have a look.

[xvitaly]

Fedora maintainer here. I cannot use Internet access during official builds. Please add solution to use pre-downloaded test data with Cmake option for example -DTEST_DATA_DIR=foo.

[nlohmann]

@xvitaly Is the approach described in #2189 (comment) feasible for you?

[xvitaly]

Is the approach described in #2189 (comment) feasible for you?

I will try it now.

[xvitaly]

@nlohmann Most of tests passed, but these were failed:

The following tests FAILED:
         50 - cmake_fetch_content_configure (Failed)
         51 - cmake_fetch_content_build (Not Run)
Errors while running CTest

[xvitaly]

@nlohmann Disabled broken tests. Added a temporary workaround to our package.

[nlohmann]

@xvitaly Thanks for reporting. Can you provide the output of the failed tests? Which CMake version are you using?

[nlohmann]

Would f4c4bab fix your issue?

[xvitaly]

Would f4c4bab fix your issue?

It works, but still require Git due to find_package(Git). Please move it under else() too.

cmake_fetch_content_configure and cmake_fetch_content_build are still broken.

[xvitaly]

Can you provide the output of the failed tests?

50/51 Test #50: cmake_fetch_content_configure ......***Failed    0.36 sec
-- The CXX compiler identification is GNU 10.1.1
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ - works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
[1/9] Creating directories for 'json-populate'
[1/9] Performing download step (git clone) for 'json-populate'
fatal: repository '/builddir/build/BUILD/json-3.8.0/test/cmake_fetch_content/project/../../..' does not exist
fatal: repository '/builddir/build/BUILD/json-3.8.0/test/cmake_fetch_content/project/../../..' does not exist
fatal: repository '/builddir/build/BUILD/json-3.8.0/test/cmake_fetch_content/project/../../..' does not exist
-- Had to git clone more than once:
          3 times.
CMake Error at json-subbuild/json-populate-prefix/tmp/json-populate-gitclone.cmake:31 (message):
  Failed to clone repository:
  '/builddir/build/BUILD/json-3.8.0/test/cmake_fetch_content/project/../../..'


FAILED: json-populate-prefix/src/json-populate-stamp/json-populate-download 
cd /builddir/build/BUILD/json-3.8.0/x86_64-redhat-linux-gnu/test/cmake_fetch_content/_deps && /usr/bin/cmake -P /builddir/build/BUILD/json-3.8.0/x86_64-redhat-linux-gnu/test/cmake_fetch_content/_deps/json-subbuild/json-populate-prefix/tmp/json-populate-gitclone.cmake && /usr/bin/cmake -E touch /builddir/build/BUILD/json-3.8.0/x86_64-redhat-linux-gnu/test/cmake_fetch_content/_deps/json-subbuild/json-populate-prefix/src/json-populate-stamp/json-populate-download
ninja: build stopped: subcommand failed.

CMake Error at /usr/share/cmake/Modules/FetchContent.cmake:912 (message):
  Build step for json failed: 1
Call Stack (most recent call first):
  /usr/share/cmake/Modules/FetchContent.cmake:1003 (__FetchContent_directPopulate)
  CMakeLists.txt:13 (FetchContent_Populate)


-- Configuring incomplete, errors occurred!
See also "/builddir/build/BUILD/json-3.8.0/x86_64-redhat-linux-gnu/test/cmake_fetch_content/CMakeFiles/CMakeOutput.log".

      Start 51: cmake_fetch_content_build
Failed test dependencies: cmake_fetch_content_configure
51/51 Test #51: cmake_fetch_content_build ..........***Not Run   0.00 sec

96% tests passed, 2 tests failed out of 51

Which CMake version are you using?

cmake-3.17.3-1.fc32.x86_64

[nlohmann]

4d96f4c moves the find_package(Git) and tries to fix the cmake_fetch_content_configure test. Please check.

[xvitaly]

moves the find_package(Git) and tries to fix the cmake_fetch_content_configure test. Please check.

Now it builds fine without installed Git. Thanks.

But tests cmake_fetch_content_configure and cmake_fetch_content_build are still broken due to missing Git and absent Internet access. New logs:

50/51 Test #50: cmake_fetch_content_configure ......***Failed    0.34 sec
-- The CXX compiler identification is GNU 10.1.1
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ - works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at /usr/share/cmake/Modules/ExternalProject.cmake:2454 (message):
  error: could not find git for clone of json-populate
Call Stack (most recent call first):
  /usr/share/cmake/Modules/ExternalProject.cmake:3271 (_ep_add_download_command)
  CMakeLists.txt:13 (ExternalProject_Add)


-- Configuring incomplete, errors occurred!
See also "/builddir/build/BUILD/json-3.8.0/x86_64-redhat-linux-gnu/test/cmake_fetch_content/_deps/json-subbuild/CMakeFiles/CMakeOutput.log".

CMake Error at /usr/share/cmake/Modules/FetchContent.cmake:900 (message):
  CMake step for json failed: 1
Call Stack (most recent call first):
  /usr/share/cmake/Modules/FetchContent.cmake:1003 (__FetchContent_directPopulate)
  CMakeLists.txt:12 (FetchContent_Populate)


-- Configuring incomplete, errors occurred!
See also "/builddir/build/BUILD/json-3.8.0/x86_64-redhat-linux-gnu/test/cmake_fetch_content/CMakeFiles/CMakeOutput.log".

      Start 51: cmake_fetch_content_build
Failed test dependencies: cmake_fetch_content_configure
51/51 Test #51: cmake_fetch_content_build ..........***Not Run   0.00 sec

96% tests passed, 2 tests failed out of 51

@nlohmann Just disable these tests if the JSON_TestDataDirectory variable is set.

[nlohmann]

I don't like the implicit dependency between "here is a directory of tests" and "don't execute this specific CMake test". I added a label git_required to the tests that would require a git checkout, see e86b3fa. You can skip these with

ctest -LE git_required

Please let me know if this is feasible for you.

[xvitaly]

Please let me know if this is feasible for you.

The problem is absent Internet access, not Git at all. I can install it in buildroot, but it will not download anything from the Internet.

[nlohmann]

I do think the problem is not related to missing Internet access. The cmake_fetch_content_configure does not download a remote Git repository, but assumes the root directory of the package is part of a Git checkout. I can reproduce the problem when downloading a ZIP from GitHub which does not contain a .git directory. Then cmake_fetch_content_configure fails complaining that

fatal: repository '/Users/niels/Downloads/json-develop/test/cmake_fetch_content/project/../../..' does not exist

[nlohmann]

@xvitaly Did you test the proposal of #2189 (comment)?

[xvitaly]

Did you test the proposal of #2189 (comment)?

Yes, it works great. Thank you.

100% tests passed, 0 tests failed out of 49

[nlohmann]

@dvzrv Is the approach in #2189 (comment) also working for you? If so, I would merge this.

[dvzrv]

@nlohmann I'm using a git clone of this repository, based on a tag which is checked against your PGP signature (but a tarball of the test repository) to build.

With that setup I get:

100% tests passed, 0 tests failed out of 52

Label Time Summary:
all             = 262.70 sec*proc (43 tests)
git_required    =   3.80 sec*proc (2 tests)

Running the tests against a tarball of this repository I get:

96% tests passed, 2 tests failed out of 52

Label Time Summary:
all             = 262.42 sec*proc (43 tests)
git_required    =   0.60 sec*proc (2 tests)

With applying the ctest switch:

100% tests passed, 0 tests failed out of 50

Label Time Summary:
all    = 265.20 sec*proc (43 tests)

Just out of curiosity: What is git used for in that context? Why is it relevant for the tests, if they are already downloaded and in place?

[nlohmann]

@dvzrv Good to know!

About your question: The test cmake_fetch_content_configure checks if it's possible to call CMake's FetchContent with the current repository state. For this, we pass the root directory of the library as GIT_REPOSITORY parameter to FetchContent_Declare in test/cmake_fetch_content/project/CMakeLists.txt. This does not work if the library was not checked out via Git. This test is for the maintainers and not really the user of the library.

Given that everything works for you, I will merge #2202.