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

Corrections to Windows unit tests. #2596

Merged

Conversation

SergioRAgostinho
Copy link
Member

@SergioRAgostinho SergioRAgostinho commented Nov 4, 2018

This a PR incorporating selected commits by @claudiofantacci as explained here
#2399 (comment) .

After the merge I will "race" to finish the release. @PointCloudLibrary/maintainers if there's anyone out there with a little time please merge this if the tests run normally. Be aware that AppVeyor is randomly timing out so check that one with some care.

Closes #2585

A past-the-end iterator was dereferenced.
When an exception was thrown during object construction, the destructor
calls serializeMetadataToDisk that dereferences null objects.

Simplification of the original commit 0995009
by @claudiofantacci .
The number of thread passed to OpenMP must be greater than 0.
The default for OpenMP classes is 0. With this commit, by setting 0, the
number of threads is set to the number of cores detected on the machine.

Closes PointCloudLibrary#2568
The test compiles differntly if the configuration is Release or Debug. In Debug mode, threads are required to finish their task in 15 seconds instead of 1.

Closes 2585
@taketwo
Copy link
Member

taketwo commented Nov 5, 2018

LGTM, thank you!

@taketwo taketwo merged commit 164752b into PointCloudLibrary:master Nov 5, 2018
@UnaNancyOwen
Copy link
Member

UnaNancyOwen commented Nov 6, 2018

100% tests passed! Thanks! 👍
Tests Result
2018-11-06_21h11_17

@SergioRAgostinho SergioRAgostinho deleted the fix/globaltestwin branch November 8, 2018 14:55
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