boostorg / thread

Boost.org thread module
http://boost.org/libs/thread
200 stars 161 forks source link

Update tests to consistently pass on Windows (hopefully) #178

Closed austin-beer closed 6 years ago

austin-beer commented 6 years ago

Increased the time thresholds on a bunch of tests to be more forgiving so they pass consistently on Windows.

Reduced the time thresholds on a bunch of tests that were TOO forgiving.

viboes commented 6 years ago

I was wondering if we couldn't have less constraining duration only for windows, but we can do it once we have a green CI.

austin-beer commented 6 years ago

Yes, I think that would be a good idea. Do all the tests by chance share a common include file where we could set the threshold once and then use it in each of the tests?

austin-beer commented 6 years ago

Is it possible to cancel this AppVeyor job (https://ci.appveyor.com/project/viboes/thread/build/1.0.147-feature/timespec_clocks) since it's been superseded by this one (https://ci.appveyor.com/project/viboes/thread/build/1.0.148-feature/timespec_clocks)?

viboes commented 6 years ago

I've stopped the job.

I wonder if we shoudln't print the difference. I have observed that sometimes we have less time than expected, no more. This can be due to time slicing; there is some time between the call to now and the call to wait.

austin-beer commented 6 years ago

All of the changes I've made allow the tests to take less time than expected without reporting errors. It's only when the tests take more than 200 milliseconds longer than expected that they report an error.

Unfortunately I've seen that sometimes 200 milliseconds isn't long enough. I think the issue is that AppVeyor is using heavily-loaded VMs and so the tests sometimes run even slower than they do on a normal Windows system.

austin-beer commented 6 years ago

The Travis CI failed due to a timing issue unrelated to these changes. My guess is it will pass if run again.

The AppVeyor Visual Studio failures were due to issues unrelated to these changes. It seems like AppVeyor isn't always the most reliable CI platform.

The AppVeyor MinGW failures are due to legitimate issues that we need to address, but that should probably be done in a separate PR, since they are unrelated to this PR.

I'll add code to each test in this PR to set the timeout threshold based on whether we're running on Windows.