winmm/tests: Allow more margin in the test_asyncWAVE() playback check.

Francois,

thank you for helping remove timing-caused flakiness from the tests.

I believe you forgot to test your patch in Wine(!). The test
+    ok(400 <= p2 && p2 <= 600, "%ums is not in the expected 400-600ms range\n", p2);
should fail in every version of Wine.

Ironically, the reason is mentioned in your patch:
     /* Give Wine's asynchronous thread time to start up.  Furthermore,
      * it uses 3 buffers per second, so that the positions reported
      * will be 333ms, 667ms etc. at best. */

IIRC, the first non-null position reported by Wine is 667ms, because
it successfully queues two 333ms buffers to waveOut.

I suggest adding a todo_wine plus an additional
ok(0<p2 && strcmp(buf,"2000")) that Wine won't fail.
And I'd decrease the sleep to e.g. 200 and 100 <= p2 && p2 <= 300
All those times sum up, the winmm test run long enough already.


MCIWave should use winmm:waveOutGetPosition while playing, however
the MCI devices hardly have any decent thread-safety and the construct
    if(status == MODE_PLAYING)
         waveOutGetPosition(hWave, &pos)
    else pos = stopped_pos;
is unsafe, as the player may have exited meanwhile and closed hWave.

It's a compatibility bug since day one of mciwave in git history that
the position increments 3 times per second only.  Yet so far,
no app seems to have been bothered.

Regards,
	Jörg Höhle

winmm/tests: Allow more margin in the test_asyncWAVE() playback check.

[Francois Gouget]

On Tue, 22 Jan 2013, Joerg-Cyril.Hoehle(a)t-systems.com wrote:
[...]
> I believe you forgot to test your patch in Wine(!). The test
> +    ok(400 <= p2 && p2 <= 600, "%ums is not in the expected 400-600ms range\n", p2);
> should fail in every version of Wine.
> [...]
> IIRC, the first non-null position reported by Wine is 667ms, because
> it successfully queues two 333ms buffers to waveOut.

Ouch. Right. I forgot to retest when I changed the upper limit. Actually 
I'm getting 1000ms here, which based on your comment below is probably 
because 667ms get buffered initially and then the first block get 
replaced by another 333ms block by the time the 500ms sleep completes so 
we end up with 1000ms.
> I suggest adding a todo_wine plus an additional
> ok(0<p2 && strcmp(buf,"2000")) that Wine won't fail.

Ok. I think this can still be narrower, 400-1000ms for instance (that 
should work as long as Wine does not get ahead by more than 166ms).
> And I'd decrease the sleep to e.g. 200 and 100 <= p2 && p2 <= 300
> All those times sum up, the winmm test run long enough already.

The problem with decreasing the length of the sleep is that you increase 
the allowed speed variance, here from +/-20% to +/-50%. But if you 
tighten the range then you run into timing granularity and delay issues 
again.
> MCIWave should use winmm:waveOutGetPosition while playing, however
> the MCI devices hardly have any decent thread-safety and the construct
>     if(status == MODE_PLAYING)
>          waveOutGetPosition(hWave, &pos)
>     else pos = stopped_pos;
> is unsafe, as the player may have exited meanwhile and closed hWave.

I'm lost here.


-- 
Francois Gouget <fgouget(a)codeweavers.com>

winmm/tests: Allow more margin in the test_asyncWAVE() playback check.

> > I suggest adding a todo_wine plus an additional
> > ok(0<p2 && strcmp(buf,"2000")) that Wine won't fail.
> > Ok. I think this can still be narrower, 400-1000ms for instance

I meant to have 2 tests
ok(0<p2 && strcmp(buf,"2000")) -- not narrowed, just to prevent complete breakage
todo_wine ok(100<=p2 && p2<=300) -- as narrow as you see fit