Skip to content

Commit

Permalink
Improve unit testing (Part 1) (#3380)
Browse files Browse the repository at this point in the history
* Refactor unit test creation

Add functions for creating tests and to supply test- and
standard-specific build settings.

Raises minimum CMake version to 3.13 in test directory.

json_test_add_test_for(
    <file>
    MAIN <main>
    [CXX_STANDARDS <version_number>...] [FORCE])

Given a <file> unit-foo.cpp, produces

    test-foo_cpp<version_number>

if C++ standard <version_number> is supported by the compiler and
thesource file contains JSON_HAS_CPP_<version_number>.  Use FORCE to
create the test regardless of the file containing
JSON_HAS_CPP_<version_number>.  Test targets are linked against <main>.
CXX_STANDARDS defaults to "11".

json_test_set_test_options(
    all|<tests>
    [CXX_STANDARDS all|<args>...]
    [COMPILE_DEFINITIONS <args>...]
    [COMPILE_FEATURES <args>...]
    [COMPILE_OPTIONS <args>...]
    [LINK_LIBRARIES <args>...]
    [LINK_OPTIONS <args>...])

Supply test- and standard-specific build settings.
Specify multiple tests using a list e.g., "test-foo;test-bar".

Must be called BEFORE the test is created.

* Use CMAKE_MODULE_PATH

* Don't undef some macros if JSON_TEST_KEEP_MACROS is defined

* Use JSON_TEST_KEEP_MACROS

Incidentally enables the regression tests for #2546 and #3070.

A CHECK_THROWS_WITH_AS in #3070 was disabled which is tracked in #3377
and a line in from_json(..., std_fs::path&) was marked with LCOV_EXCL_LINE.

* Add three-way comparison feature test macro

* Disable broken comparison if JSON_HAS_THREE_WAY_COMPARISON

* Fix redefinition of inline constexpr statics

Redelcaration of inline constexpr static data members in namespace scope
was deprecated in C++17. Fixes -Werror=deprecated compilation failures.

* Fix more test build failures due to missing noexcept

* CI: update cmake_flags test to use CMake 3.13 in test directory

Also change default for JSON_BuildTests option to depend on CMake
version.

* CI: turn *_CXXFLAGS into CMake lists

* CI: use JSON_TestStandards to set CXX_STANDARD

* CI: pass extra CXXFLAGS to standards tests
  • Loading branch information
falbrechtskirchinger committed Mar 24, 2022
1 parent 700b95f commit ad103e5
Show file tree
Hide file tree
Showing 20 changed files with 757 additions and 608 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/macos.yml
Expand Up @@ -38,7 +38,7 @@ jobs:
steps:
- uses: actions/checkout@v2
- name: cmake
run: cmake -S . -B build -D CMAKE_BUILD_TYPE=Debug -DJSON_BuildTests=On -DCMAKE_CXX_STANDARD=${{ matrix.standard }} -DCMAKE_CXX_STANDARD_REQUIRED=ON
run: cmake -S . -B build -D CMAKE_BUILD_TYPE=Debug -DJSON_BuildTests=On -DJSON_TestStandards=${{ matrix.standard }}
- name: build
run: cmake --build build --parallel 10
- name: test
Expand Down
11 changes: 9 additions & 2 deletions CMakeLists.txt
Expand Up @@ -19,6 +19,7 @@ endif()
## INCLUDE
##
##
set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake ${CMAKE_MODULE_PATH})
include(ExternalProject)

##
Expand All @@ -30,7 +31,13 @@ if (POLICY CMP0077)
cmake_policy(SET CMP0077 NEW)
endif ()

option(JSON_BuildTests "Build the unit tests when BUILD_TESTING is enabled." ${MAIN_PROJECT})
# VERSION_GREATER_EQUAL is not available in CMake 3.1
if(${MAIN_PROJECT} AND (${CMAKE_VERSION} VERSION_EQUAL 3.13 OR ${CMAKE_VERSION} VERSION_GREATER 3.13))
set(JSON_BuildTests_INIT ON)
else()
set(JSON_BuildTests_INIT OFF)
endif()
option(JSON_BuildTests "Build the unit tests when BUILD_TESTING is enabled." ${JSON_BuildTests_INIT})
option(JSON_CI "Enable CI build targets." OFF)
option(JSON_Diagnostics "Use extended diagnostic messages." OFF)
option(JSON_ImplicitConversions "Enable implicit conversions." ON)
Expand All @@ -39,7 +46,7 @@ option(JSON_MultipleHeaders "Use non-amalgamated version of the library." OF
option(JSON_SystemInclude "Include as system headers (skip for clang-tidy)." OFF)

if (JSON_CI)
include(cmake/ci.cmake)
include(ci)
endif ()

##
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/CMakeLists.txt
Expand Up @@ -23,7 +23,7 @@ if(NOT benchmark_POPULATED)
endif()

# download test data
include(${CMAKE_SOURCE_DIR}/../cmake/download_test_data.cmake)
include(download_test_data)

# benchmark binary
add_executable(json_benchmarks src/benchmarks.cpp)
Expand Down
703 changes: 360 additions & 343 deletions cmake/ci.cmake

Large diffs are not rendered by default.

204 changes: 204 additions & 0 deletions cmake/test.cmake
@@ -0,0 +1,204 @@
set(_json_test_cmake_list_file ${CMAKE_CURRENT_LIST_FILE})

#############################################################################
# download test data
#############################################################################

include(download_test_data)

# test fixture to download test data
add_test(NAME "download_test_data" COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR}
--target download_test_data
)
set_tests_properties(download_test_data PROPERTIES FIXTURES_SETUP TEST_DATA)

if(JSON_Valgrind)
find_program(CMAKE_MEMORYCHECK_COMMAND valgrind)
message(STATUS "Executing test suite with Valgrind (${CMAKE_MEMORYCHECK_COMMAND})")
set(memcheck_command "${CMAKE_MEMORYCHECK_COMMAND} ${CMAKE_MEMORYCHECK_COMMAND_OPTIONS} --error-exitcode=1 --leak-check=full")
separate_arguments(memcheck_command)
endif()

#############################################################################
# detect standard support
#############################################################################

# C++11 is the minimum required
set(compiler_supports_cpp_11 TRUE)

foreach(feature ${CMAKE_CXX_COMPILE_FEATURES})
if (${feature} STREQUAL cxx_std_14)
set(compiler_supports_cpp_14 TRUE)
elseif (${feature} STREQUAL cxx_std_17)
set(compiler_supports_cpp_17 TRUE)
elseif (${feature} STREQUAL cxx_std_20)
set(compiler_supports_cpp_20 TRUE)
elseif (${feature} STREQUAL cxx_std_23)
set(compiler_supports_cpp_23 TRUE)
endif()
endforeach()

#############################################################################
# test functions
#############################################################################

#############################################################################
# json_test_set_test_options(
# all|<tests>
# [CXX_STANDARDS all|<args>...]
# [COMPILE_DEFINITIONS <args>...]
# [COMPILE_FEATURES <args>...]
# [COMPILE_OPTIONS <args>...]
# [LINK_LIBRARIES <args>...]
# [LINK_OPTIONS <args>...])
#
# Supply test- and standard-specific build settings.
# Specify multiple tests using a list e.g., "test-foo;test-bar".
#
# Must be called BEFORE the test is created.
#############################################################################

function(json_test_set_test_options tests)
cmake_parse_arguments(args "" ""
"CXX_STANDARDS;COMPILE_DEFINITIONS;COMPILE_FEATURES;COMPILE_OPTIONS;LINK_LIBRARIES;LINK_OPTIONS"
${ARGN})

if(NOT args_CXX_STANDARDS)
set(args_CXX_STANDARDS "all")
endif()

foreach(test ${tests})
if("${test}" STREQUAL "all")
set(test "")
endif()

foreach(cxx_standard ${args_CXX_STANDARDS})
if("${cxx_standard}" STREQUAL "all")
if("${test}" STREQUAL "")
message(FATAL_ERROR "Not supported. Change defaults in: ${_json_test_cmake_list_file}")
endif()
set(test_interface _json_test_interface_${test})
else()
set(test_interface _json_test_interface_${test}_cpp_${cxx_standard})
endif()

if(NOT TARGET ${test_interface})
add_library(${test_interface} INTERFACE)
endif()

target_compile_definitions(${test_interface} INTERFACE ${args_COMPILE_DEFINITIONS})
target_compile_features(${test_interface} INTERFACE ${args_COMPILE_FEATURES})
target_compile_options(${test_interface} INTERFACE ${args_COMPILE_OPTIONS})
target_link_libraries (${test_interface} INTERFACE ${args_LINK_LIBRARIES})
target_link_options(${test_interface} INTERFACE ${args_LINK_OPTIONS})
endforeach()
endforeach()
endfunction()

# for internal use by json_test_add_test_for()
function(_json_test_add_test test_name file main cxx_standard)
set(test_target ${test_name}_cpp${cxx_standard})

if(TARGET ${test_target})
message(FATAL_ERROR "Target ${test_target} has already been added.")
endif()

add_executable(${test_target} ${file})
target_link_libraries(${test_target} PRIVATE ${main})

# set and require C++ standard
set_target_properties(${test_target} PROPERTIES
CXX_STANDARD ${cxx_standard}
CXX_STANDARD_REQUIRED ON
)

# apply standard-specific build settings
if(TARGET _json_test_interface__cpp_${cxx_standard})
target_link_libraries(${test_target} PRIVATE _json_test_interface__cpp_${cxx_standard})
endif()

# apply test-specific build settings
if(TARGET _json_test_interface_${test_name})
target_link_libraries(${test_target} PRIVATE _json_test_interface_${test_name})
endif()

# apply test- and standard-specific build settings
if(TARGET _json_test_interface_${test_name}_cpp_${cxx_standard})
target_link_libraries(${test_target} PRIVATE
_json_test_interface_${test_name}_cpp_${cxx_standard}
)
endif()

if (JSON_FastTests)
add_test(NAME ${test_target}
COMMAND ${test_target} ${DOCTEST_TEST_FILTER}
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
)
else()
add_test(NAME ${test_target}
COMMAND ${test_target} ${DOCTEST_TEST_FILTER} --no-skip
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
)
endif()
set_tests_properties(${test_target} PROPERTIES LABELS "all" FIXTURES_REQUIRED TEST_DATA)

if(JSON_Valgrind)
add_test(NAME ${test_target}_valgrind
COMMAND ${memcheck_command} $<TARGET_FILE:${test_target}> ${DOCTEST_TEST_FILTER}
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
)
set_tests_properties(${test_target}_valgrind PROPERTIES
LABELS "valgrind" FIXTURES_REQUIRED TEST_DATA
)
endif()
endfunction()

#############################################################################
# json_test_add_test_for(
# <file>
# MAIN <main>
# [CXX_STANDARDS <version_number>...] [FORCE])
#
# Given a <file> unit-foo.cpp, produces
#
# test-foo_cpp<version_number>
#
# if C++ standard <version_number> is supported by the compiler and the
# source file contains JSON_HAS_CPP_<version_number>.
# Use FORCE to create the test regardless of the file containing
# JSON_HAS_CPP_<version_number>.
# Test targets are linked against <main>.
# CXX_STANDARDS defaults to "11".
#############################################################################

function(json_test_add_test_for file)
cmake_parse_arguments(args "FORCE" "MAIN" "CXX_STANDARDS" ${ARGN})

get_filename_component(file_basename ${file} NAME_WE)
string(REGEX REPLACE "unit-([^$]+)" "test-\\1" test_name ${file_basename})

if("${args_MAIN}" STREQUAL "")
message(FATAL_ERROR "Required argument MAIN <main> missing.")
endif()

if("${args_CXX_STANDARDS}" STREQUAL "")
set(args_CXX_STANDARDS 11)
endif()

file(READ ${file} file_content)
foreach(cxx_standard ${args_CXX_STANDARDS})
if(NOT compiler_supports_cpp_${cxx_standard})
continue()
endif()

# add unconditionally if C++11 (default) or forced
if(NOT ("${cxx_standard}" STREQUAL 11 OR args_FORCE))
string(FIND "${file_content}" JSON_HAS_CPP_${cxx_standard} has_cpp_found)
if(${has_cpp_found} EQUAL -1)
continue()
endif()
endif()

_json_test_add_test(${test_name} ${file} ${args_MAIN} ${cxx_standard})
endforeach()
endfunction()
3 changes: 2 additions & 1 deletion include/nlohmann/detail/conversions/from_json.hpp
Expand Up @@ -464,7 +464,8 @@ void from_json(const BasicJsonType& j, std_fs::path& p)
{
if (JSON_HEDLEY_UNLIKELY(!j.is_string()))
{
JSON_THROW(type_error::create(302, "type must be string, but is " + std::string(j.type_name()), j));
// Not tested because of #3377 (related #3070)
JSON_THROW(type_error::create(302, "type must be string, but is " + std::string(j.type_name()), j)); // LCOV_EXCL_LINE
}
p = *j.template get_ptr<const typename BasicJsonType::string_t*>();
}
Expand Down
9 changes: 9 additions & 0 deletions include/nlohmann/detail/macro_scope.hpp
Expand Up @@ -97,6 +97,15 @@
#define JSON_HAS_FILESYSTEM 0
#endif

#ifndef JSON_HAS_THREE_WAY_COMPARISON
#if defined(__cpp_lib_three_way_comparison) && __cpp_lib_three_way_comparison >= 201907L \
&& defined(__cpp_impl_three_way_comparison)&& __cpp_impl_three_way_comparison >= 201907L
#define JSON_HAS_THREE_WAY_COMPARISON 1
#else
#define JSON_HAS_THREE_WAY_COMPARISON 0
#endif
#endif

// disable documentation warnings on clang
#if defined(__clang__)
#pragma clang diagnostic push
Expand Down
20 changes: 12 additions & 8 deletions include/nlohmann/detail/macro_unscope.hpp
Expand Up @@ -8,19 +8,23 @@
// clean up
#undef JSON_ASSERT
#undef JSON_INTERNAL_CATCH
#undef JSON_CATCH
#undef JSON_THROW
#undef JSON_TRY
#undef JSON_PRIVATE_UNLESS_TESTED
#undef JSON_HAS_CPP_11
#undef JSON_HAS_CPP_14
#undef JSON_HAS_CPP_17
#undef JSON_HAS_CPP_20
#undef JSON_HAS_FILESYSTEM
#undef JSON_HAS_EXPERIMENTAL_FILESYSTEM
#undef NLOHMANN_BASIC_JSON_TPL_DECLARATION
#undef NLOHMANN_BASIC_JSON_TPL
#undef JSON_EXPLICIT
#undef NLOHMANN_CAN_CALL_STD_FUNC_IMPL

#ifndef JSON_TEST_KEEP_MACROS
#undef JSON_CATCH
#undef JSON_TRY
#undef JSON_HAS_CPP_11
#undef JSON_HAS_CPP_14
#undef JSON_HAS_CPP_17
#undef JSON_HAS_CPP_20
#undef JSON_HAS_FILESYSTEM
#undef JSON_HAS_EXPERIMENTAL_FILESYSTEM
#undef JSON_HAS_THREE_WAY_COMPARISON
#endif

#include <nlohmann/thirdparty/hedley/hedley_undef.hpp>
8 changes: 6 additions & 2 deletions include/nlohmann/detail/meta/cpp_future.hpp
Expand Up @@ -147,8 +147,12 @@ struct static_const
static constexpr T value{};
};

template<typename T>
constexpr T static_const<T>::value; // NOLINT(readability-redundant-declaration)
#ifndef JSON_HAS_CPP_17

template<typename T>
constexpr T static_const<T>::value; // NOLINT(readability-redundant-declaration)

#endif

} // namespace detail
} // namespace nlohmann

0 comments on commit ad103e5

Please sign in to comment.