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

Split test-unicode to avoid timeouts #1884

Closed
wants to merge 2 commits into from

Conversation

nickaein
Copy link
Contributor

@nickaein nickaein commented Dec 29, 2019

This PR splits the long-running unit-test_all into two standalone unit-tests.

Fixes #1816, fixes #1032 (hopefully).


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
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 9b0a3e6 on nickaein:fix/split-unicode-tests into bde5712 on nlohmann:develop.

@nickaein
Copy link
Contributor Author

@nlohmann

What do you think of this simple re-organization for test-unicode to partition its running time that could help fix issues like #1816 and #1032.

The pipeline has falied due to (ironically) exceeding Appveyor 1 hour limit. A re-run might fix that.

I've also tried to some micro-optimizations on implementation of check_utf8dump and check_utf8string but didn't find a significant improvement (yet).

@stale
Copy link

stale bot commented Feb 11, 2020

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.

@stale 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 Feb 11, 2020
@stale stale bot closed this Feb 18, 2020
@dota17
Copy link
Contributor

dota17 commented Apr 7, 2020

@nickaein
Currently, the test-unicode_all is always not finished because times out.
This often happens in Xcode 9.2 C++ and before.

I think this PR is a good try. We can reopen this one to solve the problem.

@nickaein
Copy link
Contributor Author

@dota17 Can you test this branch locally to see whether it is effective in your case?

https://github.com/nickaein/nlohmann-json/tree/fix/split-unicode-tests

@nlohmann 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 Apr 11, 2020
@nlohmann nlohmann reopened this Apr 11, 2020
@nlohmann
Copy link
Owner

Can you please update to the latest develop branch - there have been several fixes with respect to the CI.

@dota17
Copy link
Contributor

dota17 commented Apr 14, 2020

Can you test this branch locally to see whether it is effective in your case?
https://github.com/nickaein/nlohmann-json/tree/fix/split-unicode-tests

Everything is fine in my local env.
It should be verified in this repo's travis-ci and currently, i had not find any error.

@stale
Copy link

stale bot commented May 15, 2020

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.

@stale 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 May 15, 2020
@stale stale bot closed this May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Semi-frequent timeouts in test-unicode_all with 3.6.1 (aarch64) Some testcases fail and one never finishes
4 participants