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

Travis doesn't run any tests in C++17 mode #2540

Merged
merged 5 commits into from
Dec 21, 2020

Conversation

karzhenkov
Copy link
Contributor

This is essentially not PR but an issue (for now).
All tests related to C++17 are intentionally broken here, but Travis doesn't report any errors.
A possible solution was discussed earlier in #2364 (along with a fix for std::experimental namespace).

@coveralls
Copy link

coveralls commented Dec 19, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 9493f4b on karzhenkov:disrupt-cxx17-tests into c3c574c on nlohmann:develop.

@karzhenkov
Copy link
Contributor Author

After the test configuration is fixed, Travis will catch the intentionally broken tests.

@karzhenkov
Copy link
Contributor Author

With the corrected test configuration, Travis runs tests in C++17 mode and reports injected errors, for example, here.
Now let's restore the intentionally broken tests to make them work as intended. We will probably see some new errors.

@karzhenkov
Copy link
Contributor Author

To summarize the above:

commit summary
8247a21 The original Travis configuration leaves C++17 tests not executed. The injected errors don't cause Travis to fail.
aae0e49 The corrected Travis configuration allows the intentionally broken C++17 tests to be catched . By the way, here are also catched some other errors known from #2364.
39b8d6b If the injected errors are then removed, Travis still reports others. The remaining errors relate to clang++-7.

@karzhenkov
Copy link
Contributor Author

karzhenkov commented Dec 21, 2020

With the last one-bit improvement the issue is eventually solved)
For test-conversions on clang++-7 with CXXFLAGS=-std=c++1z Travis previously reported:

18: [doctest] assertions: 294 | 294 passed | 0 failed |

Now the result is as follows:

18: [doctest] assertions: 311 | 311 passed | 0 failed |

@karzhenkov karzhenkov marked this pull request as ready for review December 21, 2020 15:12
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 Dec 21, 2020
@nlohmann nlohmann added this to the Release 3.9.2 milestone Dec 21, 2020
@nlohmann nlohmann merged commit f78d456 into nlohmann:develop Dec 21, 2020
@karzhenkov karzhenkov deleted the disrupt-cxx17-tests branch December 22, 2020 18:25
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.

3 participants