Build tests with warnings as errors enabled #2245

t-b · 2020-07-06T20:45:47Z

I'm trying to resurrect #1900.

Cleanup commits
#if DOCTEST_CLANG >= -> #if DOCTEST_CLANG && DOCTEST_CLANG >=
Fix nodiscard attribute is used for clang 3.9.1 when used via HEDLEY_WARN_UNUSED_RESULT but shouldn't nemequ/hedley#40 upstream
Get it working

nlohmann · 2020-07-06T20:48:04Z

FYI: These are the flags I check regularly: https://github.com/nlohmann/json/blob/develop/Makefile#L93.

coveralls · 2020-07-07T10:12:40Z

Coverage remained the same at 100.0% when pulling e226460 on t-b:feature/build_tests_with_warnings_as_errors into d9843fc on nlohmann:develop.

include/nlohmann/thirdparty/hedley/hedley.hpp

test/src/unit-allocator.cpp

test/src/unit-items.cpp

GCC 7.0 and higher complain when compiled in release mode about the unused result of parse-like functions. This makes totally sense as these are marked with JSON_HEDLEY_WARN_UNUSED_RESULT. Store the result in a json object named _ to silence that warning.

Co-authored-by: James Moore <james.moore@veracityuk.com>

Clang on Windows comes in two flavours, one which pretens to be like GCC and one which pretends to be like MSVC. We need to distinguish these two so that we can pass the correct compiler flags. Co-authored-by: James Moore <james.moore@veracityuk.com>

This currently produces In file included from C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\include\ciso646:9: In file included from C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\include\yvals.h:9: C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\include\yvals_core.h:494:2: error: STL1000: Unexpected compiler version, expected Clang 10.0.0 or newer. #error STL1000: Unexpected compiler version, expected Clang 10.0.0 or newer. and that is very likely due to the new -pedantic flag when compiling. Let's remove this compiler as it clearly is not meant to be used with these MSVC headers.

t-b · 2020-09-01T17:32:46Z

@nlohmann Ready.

nlohmann · 2020-09-02T20:09:49Z

.github/workflows/windows.yml

@@ -15,20 +15,6 @@ jobs:
    - name: test
      run: cd build ; ctest -j 10 -C Debug --exclude-regex "test-unicode" --output-on-failure

-  clang9:


Why did you remove Clang 9?

The commit message explains why, see e226460.

Could the problem be that windows-latest doesn't remain the same? Newer versions of Visual studio's standard c++ library also require newer versions of clang, so when testing llvm on windows, a fixed version of clang should probably be paired with a fixed version of the VS toolchain.

nlohmann · 2020-09-02T20:10:59Z

test/src/unit-allocator.cpp

@@ -29,6 +29,10 @@ SOFTWARE.

 #include "doctest_compatibility.h"

+#if DOCTEST_CLANG && DOCTEST_CLANG >= DOCTEST_COMPILER(3, 6, 0)
+    DOCTEST_CLANG_SUPPRESS_WARNING("-Wkeyword-macro")


Do you still need this after JSON_TESTS_PRIVATE was introduced?

I'll have a look if I find time.

stale · 2020-10-04T02:35:05Z

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.

MBalszun · 2020-11-17T20:07:16Z

Is it possible to override this?
E.g. when I want to compile the tests with a new compiler (with new warnings) on ci or my local computer.

t-b · 2020-11-19T14:06:13Z

@MBalszun Not as of now. But a patch is welcome to add a cmake option JSON_WARNINGS_AS_ERRORS defaulting to ON.

stale · 2020-12-25T13:18:00Z

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.

MBalszun · 2020-12-30T19:15:19Z

So, what is the state here? Can this be merged? I see there is a merge conflict, but github doesn't show me what it is.

nlohmann · 2020-12-30T19:38:08Z

I am currently overworking the targets for the CI. The goal is to move everything from the individual Travis/AppVeyor/GitHub Actions files as well as the project's Makefile into some CMake-based targets. This includes the warning flags for GCC and Clang I linked in #2245 (comment). Then I think we only need to add a similar target for MSVC.

I will open a PR to make the proposal visible. It is not done yet, but I think it will tackle most issues described here (apart of MSVC).

nlohmann · 2020-12-30T19:40:48Z

See #2561.

MBalszun · 2020-12-31T10:39:57Z

Thanks for the update. I wasn't aware that all the warning flags for gcc and clang are set in a separate make file.

That aside: You are aware tha travis.org is being shut down and that travis.com won't provide unlimited CI time for open source projects anymore right? Or have you negotiated a deal with them?

nlohmann · 2020-12-31T13:30:31Z

I have no deal with travis. If they stop working the way they used to, I will rely on AppVeyor and GitHub Actions. In the meantime, I see whether I can get a self-hosted runner to work so that I can automate as many system-independent tests as possible.

t-b requested a review from nlohmann as a code owner July 6, 2020 20:45

t-b mentioned this pull request Jul 7, 2020

nodiscard attribute is used for clang 3.9.1 when used via HEDLEY_WARN_UNUSED_RESULT but shouldn't nemequ/hedley#40

Closed

t-b force-pushed the feature/build_tests_with_warnings_as_errors branch from 0aaf600 to f3ecb6a Compare July 7, 2020 15:06

nlohmann reviewed Jul 8, 2020

View reviewed changes

include/nlohmann/thirdparty/hedley/hedley.hpp Outdated Show resolved Hide resolved

gregmarr reviewed Jul 8, 2020

View reviewed changes

test/src/unit-allocator.cpp Show resolved Hide resolved

t-b force-pushed the feature/build_tests_with_warnings_as_errors branch 2 times, most recently from 1ad4d1a to 08f93bd Compare July 14, 2020 12:11

t-b commented Jul 14, 2020

View reviewed changes

test/src/unit-items.cpp Show resolved Hide resolved

t-b force-pushed the feature/build_tests_with_warnings_as_errors branch 2 times, most recently from 727d993 to 49844f2 Compare August 5, 2020 14:39

t-b force-pushed the feature/build_tests_with_warnings_as_errors branch 4 times, most recently from 14d56b7 to 1d15d7c Compare September 1, 2020 10:20

t-b and others added 7 commits September 1, 2020 13:22

Tests: Ignore some compiler warnings

e2889e3

Co-authored-by: James Moore <james.moore@veracityuk.com>

unit-allocator.cpp: Silence compiler warning on MSVC 2015

0771226

unit-deserialization.cpp: Silence MSVC compiler warning

f907c2a

unit-items.cpp: Silence clang compiler warnings

435c7f8

t-b force-pushed the feature/build_tests_with_warnings_as_errors branch from 1d15d7c to e226460 Compare September 1, 2020 11:48

t-b requested a review from nlohmann September 1, 2020 11:49

t-b mentioned this pull request Sep 2, 2020

Always compile tests with all warnings enabled and error out on warnings #1798

Closed

nlohmann requested changes Sep 2, 2020

View reviewed changes

stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Oct 4, 2020

nlohmann removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Oct 5, 2020

stale bot added state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated and removed state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated labels Dec 25, 2020

nlohmann mentioned this pull request Dec 30, 2020

CI targets #2561

Merged

nlohmann closed this Jan 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build tests with warnings as errors enabled #2245

Build tests with warnings as errors enabled #2245

t-b commented Jul 6, 2020 •

edited

Loading

nlohmann commented Jul 6, 2020

coveralls commented Jul 7, 2020 •

edited

Loading

t-b commented Sep 1, 2020

nlohmann Sep 2, 2020

t-b Sep 2, 2020

MBalszun Dec 30, 2020

nlohmann Sep 2, 2020

t-b Sep 2, 2020

stale bot commented Oct 4, 2020

MBalszun commented Nov 17, 2020

t-b commented Nov 19, 2020

stale bot commented Dec 25, 2020

MBalszun commented Dec 30, 2020

nlohmann commented Dec 30, 2020

nlohmann commented Dec 30, 2020

MBalszun commented Dec 31, 2020

nlohmann commented Dec 31, 2020

Build tests with warnings as errors enabled #2245

Build tests with warnings as errors enabled #2245

Conversation

t-b commented Jul 6, 2020 • edited Loading

nlohmann commented Jul 6, 2020

coveralls commented Jul 7, 2020 • edited Loading

t-b commented Sep 1, 2020

nlohmann Sep 2, 2020

Choose a reason for hiding this comment

t-b Sep 2, 2020

Choose a reason for hiding this comment

MBalszun Dec 30, 2020

Choose a reason for hiding this comment

nlohmann Sep 2, 2020

Choose a reason for hiding this comment

t-b Sep 2, 2020

Choose a reason for hiding this comment

stale bot commented Oct 4, 2020

MBalszun commented Nov 17, 2020

t-b commented Nov 19, 2020

stale bot commented Dec 25, 2020

MBalszun commented Dec 30, 2020

nlohmann commented Dec 30, 2020

nlohmann commented Dec 30, 2020

MBalszun commented Dec 31, 2020

nlohmann commented Dec 31, 2020

t-b commented Jul 6, 2020 •

edited

Loading

coveralls commented Jul 7, 2020 •

edited

Loading