Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve unit testing (Part 1) #3380

Merged

Conversation

falbrechtskirchinger
Copy link
Contributor

@falbrechtskirchinger falbrechtskirchinger commented Mar 6, 2022

This PR refactors the CMake code for creating unit tests (see commit message cac4e9f).

It also removes duplicate macro definitions from unit tests and reuses the ones provided by json.hpp (see commit message 02138cf). To that end the macro JSON_TEST_KEEP_MACROS is introduced which keeps some macros from being undefined in macro_unscope.hpp.

A new CMake cache variable JSON_TestStandards is added to replace -DCMAKE_CXX_STANDARD=<version> -DCMAKE_CXX_STANDARD_REQUIRED=ON.

CMake 3.13 has been added to the ci_cmake_flags test.

Various compilation issues have been fixed. (These could be moved into a separate PR, if desired.)

As described in #3384, some tests had to be disabled. These failures are not caused by this PR but uncovered, since it enables some previously untested configurations.

Documentation may need updating.

@falbrechtskirchinger
Copy link
Contributor Author

falbrechtskirchinger commented Mar 6, 2022

Reminder to myself to document that overrides need to be declared before adding the to-be-overridden tests.
Done.

@coveralls
Copy link

coveralls commented Mar 6, 2022

Coverage Status

Coverage remained the same at 100.0% when pulling 3419363 on falbrechtskirchinger:topic/test-improvements-part1 into 700b95f on nlohmann:develop.

@falbrechtskirchinger
Copy link
Contributor Author

falbrechtskirchinger commented Mar 6, 2022

Coverage decrease is due to this disabled check: #3377 Fixed.

@falbrechtskirchinger falbrechtskirchinger force-pushed the topic/test-improvements-part1 branch 3 times, most recently from 0dbab35 to 04b4a29 Compare March 6, 2022 16:19
@falbrechtskirchinger falbrechtskirchinger force-pushed the topic/test-improvements-part1 branch 3 times, most recently from 3920370 to ec60320 Compare March 9, 2022 12:43
@falbrechtskirchinger falbrechtskirchinger marked this pull request as ready for review March 9, 2022 15:33
@falbrechtskirchinger falbrechtskirchinger changed the title [WIP] Improve unit testing (Part 1) Improve unit testing (Part 1) Mar 9, 2022
@falbrechtskirchinger falbrechtskirchinger marked this pull request as draft March 12, 2022 14:30
@falbrechtskirchinger falbrechtskirchinger marked this pull request as ready for review March 13, 2022 09:29
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.
Incidentally enables the regression tests for nlohmann#2546 and nlohmann#3070.

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

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some first comments - I am not done yet reviewing cmake/test.cmake and test/CMakeLists.txt.

cmake/ci.cmake Show resolved Hide resolved
include/nlohmann/detail/meta/cpp_future.hpp Show resolved Hide resolved
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@nlohmann nlohmann self-assigned this Mar 24, 2022
@nlohmann nlohmann added this to the Release 3.10.6 milestone Mar 24, 2022
@nlohmann nlohmann merged commit ad103e5 into nlohmann:develop Mar 24, 2022
@nlohmann
Copy link
Owner

Thanks!

@falbrechtskirchinger falbrechtskirchinger deleted the topic/test-improvements-part1 branch March 24, 2022 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants