Regression tests for MSVC

[nickaein]

Fixes #1543 (and avoid regression to issues like #1531 and #1536).
Fixes #1608.

This pull request:

1. Adds Debug builds on Appveyor for VS2015 and VS2017.

2. Add a separate build job that first injects #include <Windows.h> line to the top of json.hpp header. This build job helps to detect whether the presence of Windows.h could have any undersiable side-effects.

On point 2: Currently the unit-test for case 2 doesn't do much and only can catch compile errors. Also, I tested it on my json.hpp with some unfixed numeric_limits<>::max() and numeric_limits<>min() functions and it only caused compiler errors on Debug builds. Probably on Release builds, the compiler eliminates the codes related to unfixed macro definitions. Therefore, the unit-test only can catch compiler errors on Debug mode.

A more thorough approach is that instead of adding a unit-test with #include <Windows.h> on top, we add a separate build configuration in which the #include <Windows.h> gets injected to the top of json.hpp. In this build, all codes and unit-tests will be complied and checked in presence of Windows.h.

I have implemented and tested the second approach on my fork but and wanted to double check and get some feedback before adding it to the PR.

---

Pull request checklist

Read the Contribution Guidelines for detailed information.

• Changes are described in the pull request, or an existing issue is referenced.
• The test suite compiles and runs without error.
• Code coverage is 100%. Test cases can be added by editing the test suite.
• The source code is amalgamated; that is, after making changes to the sources in the include/nlohmann directory, run make amalgamate to create the single-header file single_include/nlohmann/json.hpp. The whole process is described here.

Please don't

• The C++11 support varies between different compilers and versions. Please note the list of supported compilers. Some compilers like GCC 4.7 (and earlier), Clang 3.3 (and earlier), or Microsoft Visual Studio 13.0 and earlier are known not to work due to missing or incomplete C++11 support. Please refrain from proposing changes that work around these compiler's limitations with #ifdefs or other means.
• Specifically, I am aware of compilation problems with Microsoft Visual Studio (there even is an issue label for these kind of bugs). I understand that even in 2016, complete C++11 support isn't there yet. But please also understand that I do not want to drop features or uglify the code just to make Microsoft's sub-standard compiler happy. The past has shown that there are ways to express the functionality such that the code compiles with the most recent MSVC - unfortunately, this is not the main objective of the project.
• Please refrain from proposing changes that would break JSON conformance. If you propose a conformant extension of JSON to be supported by the library, please motivate this extension.
• Please do not open pull requests that address multiple issues.

[coveralls]

Coverage remained the same at 100.0% when pulling 5b2e230 on nickaein:msvc-regression-tests into 39011a1 on nlohmann:develop.

[nlohmann]

The AppVeyor tests seem to be broken. Any idea on this?

[nickaein]

There were two errors:

1. The test-unicode_all was timing out due to slower binary in Debug mode. I've pushed a fix to increase the timeout for this particular unit-test on Debug build.

2. The second error, which is still present, is unit-concepts failing on this check:

CHECK(std::is_nothrow_copy_constructible<json::const_iterator>::value)

By borrowing build flags for VS2015-Debug it can be reproduced: https://godbolt.org/z/MOkndK

It seems that /MDd flag causes this error. This could be similar to #1536 where the C++ compiler on VS2015 added some definitions in debug mode. This error only happens on VS2015 (msvc 19.0) Debug build (where /MDd is present) and doesn't happen in Release build and/or later compiler versions.

That's how far I was be able to get. Maybe @mistersandman or @theodelrieu could shed some light on what is causing json::const_iterator to fail is_nothrow_copy_constructible criteria when we are building on debug mode (msvc 19.0).

[nickaein]

Also, do you have any suggestion on the choices in #1570 (comment) to have a build with Windows.h?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[nlohmann]

Hey @nickaein! Thanks for the PR and sorry for not having it checked earlier. I only have some minor remarks, and it would be great if you could address them.

[nickaein]

@nlohmann: Hey Niels! No problem at all. This was blocked until yesterway anyway.

The build seems to be passing. While reviewing, you might take a closer look at 4c23af1 which is intended to define copy-constructor for const_iterator explicitly. Is it OK?

[nlohmann]

Another minor comment.

[nlohmann]

Now the latest CI build at AppVeyor is failing again... Either we set up the timeout or we only use the small test suite in the Debug build. What do you think?

[nickaein]

In the last succesful CI build, MSVC debug builds has taken ~1 hour. It seems we are almost exceeding the 1 hour time restriction of AppVeyor.

As you suggested, we can skip time-consuimg test(s) for MSVC debug, at least test-unicode --all which seems to be the worst offender. Since they still will be run in other build jobs (including MSVC 2015/2017 Release mode), I think this would be a good compromise between keeping the integerity of test and speeding up (and fixing!) the CI.

[nlohmann]

Now I just wait until the CI jobs run through and we're set. Thanks!

[nlohmann]

---

🔖 Release item

This issue/PR will be part of the next release of the library. This template helps preparing the release notes.

Type

• ✨ New Feature
• 🐛 Bug Fix
• ⚡️ Improvement
• 🔨 Further Change
• 🔥 Deprecated function

Description

• Add regression tests for MSVC.
• Fix MSVC Debug build.

---

[nickaein]

Thanks!

[nlohmann]

Thank you!