To error out or to skip tests?

[Joerg-Cyril.Hoehle]

Hi,

I apologize for the numerous occurrances of red color on test.winehq
due to my new winmm MCI tests.  That was not expected.

However, turning something green is not a goal in itself.  For some
time now I wanted to query wine-devel about this topic, independently
of my patches, so here it goes.

An analysis reveals that my tests implement a detector of
vmware (virtualbox etc.) default installs, i.e. MS-Windows
systems that have not been configured with sound.

No real user's machines would be like that
(except for a w95 box I tried with a broken soundcard).

The revealing line is
> Tests skipped: Please install audio driver. Tests will fail (18 errors)
This goes hand in hand with "5 tests, 0 errors, 1 skip" in winmm:wave.
Exactly those systems skipping the wave tests produce 18-19 MCI errors.

Failure to flag an error in such cases has known drawbacks:
- For several month, Austin did not notice that his BSD boxes had lost
  sound.  wave.ok simply reports "5 tests ok, 1 skipped" in joyfull
  green, whereas it executed many tests half a year ago.

- All those vmware boxes configured without sound and thus skipping
  all but 5 wave.ok tests have for monthes *not* provided valuable
  feedback to Wine.  So they were actually *not* executing tests, when
  they could have.
  It's only when Paul tested one of my tests that he realized that
  sound was not configured on one of his vmware boxes, promptly
  configured the sound device, and the tests passed.

The picture on test.winehq.org is a disaster: None of the w95, w98, nt4,
2k, 2k3, Vista, 2k8, w7 machines have performed winmm:wave tests!
They all have no sound configured.  Mostly useless.

Therefore, I believe it's sometimes better to flag an error than to
calmly skip something.

Last year already, submitting the ICCVID patches, I was thinking:
"iccvid.dll may not part of every installation, but we want feedback
so we should compare behaviour with MS-Windows systems equipped with
that library and ask people to install it if not present".
Likewise, Wine should compare with systems similarly equipped: d3d8,
d3d9, dplay, dmusic etc.

Perhaps an easy change on test.winehq would be to augment the blue
color indicator of skipped tests to the full rectangle surface instead
of the current green?

Of course, I'll change my tests to flag a single error on systems
without sound.  -- Or no error at all, if that's wine-devel consensus.

What do you think?
     Jörg Höhle.
There's only few real MCI test errors I saw on test.winehq: the system
"fg_winxp-ie8" that yields 1 error instead of 18:
mci.c:391: Test failed: not enough time elapsed 80ms -- a flakey test.
And on the ME systems:
mci.c:603: Test failed: mci re-auto-open status mode returned error: 275
mci.c:612: Test failed: mci auto-open pause returned error: 275
Luckily, there are a few machines that pass all tests.

[Henri Verbeet]

The D3D tests have similar issues with VMs, but if it's a valid
configuration we can't let the test fail (in general).

[Greg Geldorp]

Hi Jörg

> Failure to flag an error in such cases has known drawbacks:
> - For several month, Austin did not notice that his BSD boxes had lost
>   sound.  wave.ok simply reports "5 tests ok, 1 skipped" in joyfull
>   green, whereas it executed many tests half a year ago.
>
> - All those vmware boxes configured without sound and thus skipping
>   all but 5 wave.ok tests have for monthes *not* provided valuable
>   feedback to Wine.  So they were actually *not* executing tests, when
>   they could have.
>   It's only when Paul tested one of my tests that he realized that
>   sound was not configured on one of his vmware boxes, promptly
>   configured the sound device, and the tests passed.
>
> The picture on test.winehq.org is a disaster: None of the w95, w98, nt4,
> 2k, 2k3, Vista, 2k8, w7 machines have performed winmm:wave tests!
> They all have no sound configured.  Mostly useless.

A lot of those VMware boxes are mine. They come in two flavors. Some of them (starting with esx-) are ESX VMs, ESX is VMware's data center product, which simply doesn't support sound. The others (starting with gvg-) are Server VMs, which do support sound. The reason I didn't configure sound on them is because I'm simply not interested in sound. I'll consider enabling sound, but to be honest I'm a bit put off by your aggressive tone.

Anyway, the end result is that there will always be ESX VMs that can't run sound tests.

Best regards, Ge.

[Nate Gallaher]

It sounds to me like the root problem is not that tests were being
skipped, but that developers didn't know that the test machines had no
sound configured. Perhaps simply adding -nosound to the testbed names
would clear up the issue? Then developers interested in sound can look
for testbed machines that don't have that tag and that don't skip their
tests?

Taking it to the next level I think would involve some sort of "expected
configuration" file, setup for each test machine, which you feed to the
test framework. The framework then flags particular skipped tests as
errors for that expected configuration. Then someone would have to
maintain this and make sure there's adequate coverage.  This seems like
a lot of work, and potentially a lot of spaghetti, which makes me think
#1 is the better option for now.

Regards,
~Nate

Joerg-Cyril.Hoehle@... wrote:

> Hi,
>
> I apologize for the numerous occurrances of red color on test.winehq
> due to my new winmm MCI tests.  That was not expected.
>
> However, turning something green is not a goal in itself.  For some
> time now I wanted to query wine-devel about this topic, independently
> of my patches, so here it goes.
>
> An analysis reveals that my tests implement a detector of
> vmware (virtualbox etc.) default installs, i.e. MS-Windows
> systems that have not been configured with sound.
>
> No real user's machines would be like that
> (except for a w95 box I tried with a broken soundcard).
>
> The revealing line is
>  
>> Tests skipped: Please install audio driver. Tests will fail (18 errors)
>>    
> This goes hand in hand with "5 tests, 0 errors, 1 skip" in winmm:wave.
> Exactly those systems skipping the wave tests produce 18-19 MCI errors.
>
> Failure to flag an error in such cases has known drawbacks:
> - For several month, Austin did not notice that his BSD boxes had lost
>   sound.  wave.ok simply reports "5 tests ok, 1 skipped" in joyfull
>   green, whereas it executed many tests half a year ago.
>
> - All those vmware boxes configured without sound and thus skipping
>   all but 5 wave.ok tests have for monthes *not* provided valuable
>   feedback to Wine.  So they were actually *not* executing tests, when
>   they could have.
>   It's only when Paul tested one of my tests that he realized that
>   sound was not configured on one of his vmware boxes, promptly
>   configured the sound device, and the tests passed.
>
> The picture on test.winehq.org is a disaster: None of the w95, w98, nt4,
> 2k, 2k3, Vista, 2k8, w7 machines have performed winmm:wave tests!
> They all have no sound configured.  Mostly useless.
>
> Therefore, I believe it's sometimes better to flag an error than to
> calmly skip something.
>
> Last year already, submitting the ICCVID patches, I was thinking:
> "iccvid.dll may not part of every installation, but we want feedback
> so we should compare behaviour with MS-Windows systems equipped with
> that library and ask people to install it if not present".
> Likewise, Wine should compare with systems similarly equipped: d3d8,
> d3d9, dplay, dmusic etc.
>
> Perhaps an easy change on test.winehq would be to augment the blue
> color indicator of skipped tests to the full rectangle surface instead
> of the current green?
>
> Of course, I'll change my tests to flag a single error on systems
> without sound.  -- Or no error at all, if that's wine-devel consensus.
>
> What do you think?
>      Jörg Höhle.
> There's only few real MCI test errors I saw on test.winehq: the system
> "fg_winxp-ie8" that yields 1 error instead of 18:
> mci.c:391: Test failed: not enough time elapsed 80ms -- a flakey test.
> And on the ME systems:
> mci.c:603: Test failed: mci re-auto-open status mode returned error: 275
> mci.c:612: Test failed: mci auto-open pause returned error: 275
> Luckily, there are a few machines that pass all tests.
>
>
>  

[Joerg-Cyril.Hoehle]

Ge,

> to be honest I'm a bit put off by your aggressive tone.
Please accept my apologies. It was not meant to be agressive.
I was certainly disappointed by having submitted tests that
I knew worked on 3 real machines: w95, w2k, xp, then turn to
test.winehq and see everything red.

BTW, dsound is also affected: both dsound and winmm have had little
testing coverage despite the appealing green color.

>I'm simply not interested in sound
Please weight providing as much test data as possible to the Wine
project against your lack of interest in sound issues.
BTW, the default sound tests make no noise.

>Anyway, the end result is that there will always be ESX VMs
>that can't run sound tests.
Note taken.

The question remains:
a) turn sound tests red on machines without sound (with 1 error only);
b) turn them blue (clearly marking skipped tests)
c) turn them green -- status quo (for dsound.ok and wave.ok).

Regards,
        Jörg Höhle.

[Juan Lang-4]

Hi Joerg,

> BTW, dsound is also affected: both dsound and winmm have had little
> testing coverage despite the appealing green color.

Your concern about coverage is valid one.

> Please weight providing as much test data as possible to the Wine
> project against your lack of interest in sound issues.

I'm with Ge on this one:  I don't much care whether Wine is able to
play sound.  I care in an abstract sense, but not more than I'd like
to see green tests ;-)  Is that silly or misguided of me?  Perhaps.
On the other hand, I think there's real value in stating that "the
tests should pass on all Windows boxes," even if their configuration
is invalid or strange.  We're not there yet, but any failing test
impedes that goal.

To put it another way:  when the test failures seem intractable, it's
hard to get motivated to fix them.  For example, if there are some
tests no one understands, or that fail on systems few people run, or
if the number of test failures is simply too large, it's hard to get
motivated to chip away at the issues.  Fortunately we have a few
cheerleaders like Paul who get us motivated to work on them every now
and again.

On the other hand, when there are few test failures, there can be a
little incentive to see whether a few days' effort will produce
passing tests on at least one file, in order to see it magically
become green.  There has been for me, at least.

So, to answer your question:

> a) turn sound tests red on machines without sound (with 1 error only);
> b) turn them blue (clearly marking skipped tests)
> c) turn them green -- status quo (for dsound.ok and wave.ok).

I'm in favor of either b or c, though I lean toward c.  The reason is,
not all tests can run on all systems.  For example, you mentioned you
saw a failure on a Win95 box with a broken sound card.  If the sound
card is broken, and other test results are invalid, surely a skip is
appropriate here?  That is, this doesn't indicate an error in the
tests, and skip is appropriate.  Green vs. red tells you at a glance
what the pass rate is, though it doesn't say anything about the skip
rate.

I think what you're asking for is a separate metric for coverage and
pass rate.  I'm in favor of that.
--Juan

[Reece Dunn-2]

2009/11/6  <Joerg-Cyril.Hoehle@...>:

> Ge,
>
>> to be honest I'm a bit put off by your aggressive tone.
> Please accept my apologies. It was not meant to be agressive.
> I was certainly disappointed by having submitted tests that
> I knew worked on 3 real machines: w95, w2k, xp, then turn to
> test.winehq and see everything red.
>
> BTW, dsound is also affected: both dsound and winmm have had little
> testing coverage despite the appealing green color.
>
>>I'm simply not interested in sound
> Please weight providing as much test data as possible to the Wine
> project against your lack of interest in sound issues.
> BTW, the default sound tests make no noise.
>
>>Anyway, the end result is that there will always be ESX VMs
>>that can't run sound tests.
> Note taken.
>
> The question remains:
> a) turn sound tests red on machines without sound (with 1 error only);
> b) turn them blue (clearly marking skipped tests)
> c) turn them green -- status quo (for dsound.ok and wave.ok).

 309     err = mciSendString("open waveaudio!tempfile.wav alias
mysound", buf, sizeof(buf), NULL);
 310     ok(!err,"mci open waveaudio!tempfile.wav returned error: %d\n", err);
 311     if(err) {
 312         skip("tempfile.wav was not found (not saved), skipping\n");
 313         return;
 314     }

Here, you are doing both (generating an error and skipping)!

Also, how do you know that this is due to tempfile.wav being missing
(the skip is invoked for any error case)? Especially given the line:

   mci.c:261: Tests skipped: Please install audio driver. Tests will fail

in the test results.

Shouldn't this be something like:

      err = mciSendString("open waveaudio!tempfile.wav alias mysound",
buf, sizeof(buf), NULL);
      if(err == MCIERR_INVALID_FILE) {
          skip("tempfile.wav was not found (not saved?), skipping\n");
          return;
      }
      ok(!err,"mci open waveaudio!tempfile.wav returned error: %d\n",
dbg_mcierr(err));

I think that it is safe to say that if there is no audio driver
present, all subsequent mci tests will fail and should also be skipped
(ideally with just the single warning, e.g. by returning FALSE from
that function and wrapping the other tests in an if).

  27 static const char* dbg_mcierr(MCIERROR err)
  ...
 108      default: return "unknown error";

This is not really helpful, it should contain the error number.

    mci.c:239: Test failed: mci set format tag pcm returned error: 282

This (and the other error numbers) does not mean much to me. There is
a dbg_mcierr function that was created; shouldn't that be used here
(as the error is returned from mciSendString)?

- Reece

[Joerg-Cyril.Hoehle]

Reece Dunn wrote:

>> The question remains:
>> a) turn sound tests red on machines without sound (with 1
>error only);
>> b) turn them blue (clearly marking skipped tests)
>> c) turn them green -- status quo (for dsound.ok and wave.ok).

> 310     ok(!err,"mci open waveaudio!tempfile.wav returned error: %d\n", err);
> 311     if(err) {
> 312         skip("tempfile.wav was not found (not saved), skipping\n");
>Here, you are doing both (generating an error and skipping)!

On purpose. I wanted an error flagged to point people's attention to the
issue of machines without sound, so that it can be discussed.

>I think that it is safe to say that if there is no audio driver
>present, all subsequent mci tests will fail and should also be skipped
>(ideally with just the single warning, e.g. by returning FALSE from
>that function and wrapping the other tests in an if).

My current line of thought is that broken and skip are somewhat *bad*
things, because by not performing tests, we don't learn anything about
the behaviour in "unusual" circumstances.

The relevant question becomes: when is a configuration unusual? When
can it be ignored for the purpose of Wine and when is it relevant?

- I don't care that tests fail on my father in law's machine with a
  broken sound card (or rather, the card maybe ok, but somehow the
  sndblast.dll doen't manager to install it).
  Yet testing on that machine was very revealing (black box testing
  with broken configurations tells you a lot about what's inside, like
  scientists learn a lot from patients with a local brain disease).

- Ge Geldorp pointing out that some vmware engines do not support
  sound at all renders Windows without sound not unusual -- unlike
  what I initially thought.

My conclusion is that we need tests to specifically find out how
MS-Windows behaves on such machines, so that Wine can mimick it.
That might be written entirely without skip() or broken(), because
it becomes expected behaviour.


if (wave_GetNumDevs()== 0) {
   skip("no sound, no tests\n"); return;
is *not* the answer IMHO.  Turning all lights green is not a primary
goal, it's a secondary/subordinated one.


What's the purpose of skip()? Nothing but generate blue colour in
test.winehq (use trace(); return; if all you need is print something
and avoid other tests).  What's the purpose of the blue colour?
- A call for action?
  + "please run again in the english locale so I can perform locale-dependent tests";
  + "please install sound so that you'll know whether it works on BSD"
  + "please install Gecko so I can perform more tests"
- ...?


>Shouldn't this be something like:
>      if(err == MCIERR_INVALID_FILE) {
>          skip("tempfile.wav was not found (not saved?), skipping\n");
No. If any error happens in open, we can't continue. Perhaps you mean
if (err) {
   ok(err==MCIERR_FILE_NOT_FOUND, "...\n"); /* the expected situation */
   skip("tempfile.wav was not found (not saved?), skipping\n");

>a dbg_mcierr function that was created; shouldn't that be used here
I didn't retrofit it into all messages after I wrote it. Will do so later.

Regards,
        Jörg Höhle.

[Paul Vriens-2]

On 11/09/2009 11:27 AM, Joerg-Cyril.Hoehle@... wrote:

> Reece Dunn wrote:
>
>>> The question remains:
>>> a) turn sound tests red on machines without sound (with 1
>> error only);
>>> b) turn them blue (clearly marking skipped tests)
>>> c) turn them green -- status quo (for dsound.ok and wave.ok).
>
>> 310     ok(!err,"mci open waveaudio!tempfile.wav returned error: %d\n", err);
>> 311     if(err) {
>> 312         skip("tempfile.wav was not found (not saved), skipping\n");
>> Here, you are doing both (generating an error and skipping)!
>
> On purpose. I wanted an error flagged to point people's attention to the
> issue of machines without sound, so that it can be discussed.
>
>> I think that it is safe to say that if there is no audio driver
>> present, all subsequent mci tests will fail and should also be skipped
>> (ideally with just the single warning, e.g. by returning FALSE from
>> that function and wrapping the other tests in an if).
>
> My current line of thought is that broken and skip are somewhat *bad*
> things, because by not performing tests, we don't learn anything about
> the behaviour in "unusual" circumstances.
>

There are so many variables on systems that these were introduced some
time ago. The general feeling is that we try to have succeeding tests on
a many boxes as possible. Currently we are already having a hard time
achieving that on clean/correct (so we think) boxes. Once the majority
of these clean boxes are showing up green we can start thinking of
adding tests for broken configurations.

You are of course free to have your tests succeed for every possible
combination out there, but the key word is 'succeed' here.

So if you find that we have test failures for a non-admin user with a
Transylvanian locale with no soundcard it still should show up green
(with or without the blue borders).

> What's the purpose of skip()? Nothing but generate blue colour in
> test.winehq (use trace(); return; if all you need is print something
> and avoid other tests).  What's the purpose of the blue colour?
> - A call for action?
>    + "please run again in the english locale so I can perform locale-dependent tests";
>    + "please install sound so that you'll know whether it works on BSD"
>    + "please install Gecko so I can perform more tests"
> - ...?

In my opinion the main purpose for skip() is to not run tests for
legitimate reasons. If the tests are written so that they can only be
run (aka succeed) when a sound card is present you skip() these tests on
systems without a soundcard. skip() should not be used to get everything
green (the same is in theory true for broken()).

A good example of using skip() is when functions are just not available
on a platform (like most W-calls on 9x boxes). We also have some tests
that will only succeed when the user is an administrator. It's perfectly
valid to use skip() if we find the user isn't.

Broken() is a little bit different with that respect that we usually
mark platforms/boxes as broken() when 99% (or at least the majority) of
other configurations succeed these tests.

>
> Regards,
> Jörg Höhle.

--
Cheers,

Paul.

[Kai Blin-2]

On Friday 06 November 2009 13:21:45 Joerg-Cyril.Hoehle@... wrote:

> The picture on test.winehq.org is a disaster: None of the w95, w98, nt4,
> 2k, 2k3, Vista, 2k8, w7 machines have performed winmm:wave tests!
> They all have no sound configured.  Mostly useless.

By extension of your logic, all results from Windows machines < Vista and from
most Wine boxes are useless for ws2_32 tests, as they all don't have IPv6
configured so the IPv6 tests skip. To pick up on Nate's idea, I'd request that
everybody adds a -ipv4 or -ipv6 suffix to the names of the test reports.

Additionally, no single test machine runs as part of a windows AD or even NT4
domain. Thus, a couple of advapi32 tests are skipped, making them worthless.
Perhaps people should add a -nodomain suffix to their test report names.

If I make all of those tests error out, I'll be drowning real test failures in
the noise of configuration issues. I think the same thing applies to your
sound tests. Btw, just ask the d3d folks about their woes with (almost)
everybody running their windows-based tests on virtual machines, making a lot
of the d3d tests useless. Not to mention that there's many people who run
hardware with really lousy graphics drivers. Having to skip tests because the
system's configuration does not support running them is just something we need
to deal with.

Kai

PS: I was joking about the suffixes. Just to make that clear. I don't want to
go having to call my test boxes win2k3-kb-vb-nosound-domain-noadmin-ipv6-ie7-
bluebg-classictheme-en_IE-nodotnet

--
Kai Blin
WorldForge developer  http://www.worldforge.org/
Wine developer        http://wiki.winehq.org/KaiBlin
Samba team member     http://www.samba.org/samba/team/
--
Will code for cotton.

[Joerg-Cyril.Hoehle]

Hi,

Paul Vriens wrote:
>In my opinion the main purpose for skip() is to not run tests for
>legitimate reasons.
Alas, this sentence and the examples you give do not explain to me why
to use skip instead of e.g. trace + return. What is the added value?
What to do about that blue color or mention in the log file?

My principle is: do not produce output if there's no consumer.
Refuse to write a memo if nobody is going to read it!

Why, on w95, should I use skip to mark the impossibility
to call UTF-16 xyzW functions? Why not simply use
if (W_isAvailable) { do_more_tests }
We already have files that end up performing a varying number
of tests, depending on simple if-then-else statements, without use of skip.

That's different from the localisation example: I could switch the locale,
then re-run the tests.  This is again this skip=="call for action" idea
I mentioned previously.


BTW, I've sent two patches today to stop errors on machines with sound.  The
third one (the localisation issue -- milliseconden, you already know it)
is written but currently at the end of my patch queue.  That should bring
the number of errors down to 0 on every machine with sound.

The final one, about how to deal with machines with no sound, yet trying
to test something instead of skipping everything, I still have to think about.

Regards,
        Jörg Höhle

[Reece Dunn-2]

2009/11/9  <Joerg-Cyril.Hoehle@...>:

> Hi,
>
> Paul Vriens wrote:
>>In my opinion the main purpose for skip() is to not run tests for
>>legitimate reasons.
> Alas, this sentence and the examples you give do not explain to me why
> to use skip instead of e.g. trace + return. What is the added value?
> What to do about that blue color or mention in the log file?
>
> My principle is: do not produce output if there's no consumer.
> Refuse to write a memo if nobody is going to read it!
>
> Why, on w95, should I use skip to mark the impossibility
> to call UTF-16 xyzW functions? Why not simply use
> if (W_isAvailable) { do_more_tests }
> We already have files that end up performing a varying number
> of tests, depending on simple if-then-else statements, without use of skip.
>
> That's different from the localisation example: I could switch the locale,
> then re-run the tests.  This is again this skip=="call for action" idea
> I mentioned previously.
>
>
> BTW, I've sent two patches today to stop errors on machines with sound.  The
> third one (the localisation issue -- milliseconden, you already know it)
> is written but currently at the end of my patch queue.  That should bring
> the number of errors down to 0 on every machine with sound.
>
> The final one, about how to deal with machines with no sound, yet trying
> to test something instead of skipping everything, I still have to think about.

Skipping is a standard way of saying that "we are trying to test
functionality A, but the host machine/configuration does not support
it." This is not an error (as it is a valid confgiuration/setup on
Windows machines), so producing errors only adds to the noise.

- Reece

[Reece Dunn-2]

2009/11/9  <Joerg-Cyril.Hoehle@...>:

> Reece Dunn wrote:
>
>>> The question remains:
>>> a) turn sound tests red on machines without sound (with 1
>>error only);
>>> b) turn them blue (clearly marking skipped tests)
>>> c) turn them green -- status quo (for dsound.ok and wave.ok).
>
>> 310     ok(!err,"mci open waveaudio!tempfile.wav returned error: %d\n", err);
>> 311     if(err) {
>> 312         skip("tempfile.wav was not found (not saved), skipping\n");
>>Here, you are doing both (generating an error and skipping)!
>
> On purpose. I wanted an error flagged to point people's attention to the
> issue of machines without sound, so that it can be discussed.

What has this to do with the issue of machines without sound.

The ok statement is saying that "mci open" has failed (for *any*
reason). The skip (activated on *all* error codes) is saying that
tempfile.wav is not found, even for cases such as err ==
MCIERR_CANNOT_LOAD_DRIVER!

>>I think that it is safe to say that if there is no audio driver
>>present, all subsequent mci tests will fail and should also be skipped
>>(ideally with just the single warning, e.g. by returning FALSE from
>>that function and wrapping the other tests in an if).
>
> My current line of thought is that broken and skip are somewhat *bad*
> things, because by not performing tests, we don't learn anything about
> the behaviour in "unusual" circumstances.

The point I am making is that:
  1.  The error and skip statements are saying the same thing;
  2.  But the skip message is misleading.

That is, it is valid to test MCI in the case when there is no sound
device, but the tests are being confused by a lack of tempfile.wav.

For example:
   mci.c:310: Test failed: mci open waveaudio!tempfile.wav returned error: 275
   mci.c:312: Tests skipped: tempfile.wav was not found (not saved), skipping
If I have counted the offsets correctly 275 is
MCIERR_DEVICE_NOT_READY, *not* that tempfile.wav was not found.

> The relevant question becomes: when is a configuration unusual? When
> can it be ignored for the purpose of Wine and when is it relevant?

Answer: no configuration is unusual.

> - I don't care that tests fail on my father in law's machine with a
>  broken sound card (or rather, the card maybe ok, but somehow the
>  sndblast.dll doen't manager to install it).

The point is, if the machine has a broken soundcard (or in Wine does
not have an available sound driver, or is broken due to PulseAudio on
Ubuntu), the tests should still pass (or be skipped) as is
appropriate.

>  Yet testing on that machine was very revealing (black box testing
>  with broken configurations tells you a lot about what's inside, like
>  scientists learn a lot from patients with a local brain disease).

Sure. But (a) the tests should still succeed (and doing so, document
the behaviour for both these cases) and (b) should succeed or fail
*for the right reason*.

> - Ge Geldorp pointing out that some vmware engines do not support
>  sound at all renders Windows without sound not unusual -- unlike
>  what I initially thought.

Sure.

> My conclusion is that we need tests to specifically find out how
> MS-Windows behaves on such machines, so that Wine can mimick it.
> That might be written entirely without skip() or broken(), because
> it becomes expected behaviour.

But please make sure that they are failing in a way that is expected,

It also seems to me that there should be some tests that test opening
an mci stream on a file that we know not to exist (e.g.
this-file-does-not-exist.wav) so that those failure cases can be
properly documented.

> if (wave_GetNumDevs()== 0) {
>   skip("no sound, no tests\n"); return;
> is *not* the answer IMHO.  Turning all lights green is not a primary
> goal, it's a secondary/subordinated one.

You have the basic API and parameter tests that can be done. You have
the playback tests. And you have the playback tests for non-existent
files.

In the above example, you have the case of what happens when there are
no devices available. Here, I would argue with you that skipping at
that point does not make sense. However, skipping at the
mciSendString("open ...") when you revieve an
MCIERR_CANNOT_LOAD_DRIVER (or whatever the error code is that is
returned), *then* you skip (saying "unable to load the mci driver").

> What's the purpose of skip()? Nothing but generate blue colour in
> test.winehq (use trace(); return; if all you need is print something
> and avoid other tests).  What's the purpose of the blue colour?
> - A call for action?
>  + "please run again in the english locale so I can perform locale-dependent tests";
>  + "please install sound so that you'll know whether it works on BSD"
>  + "please install Gecko so I can perform more tests"
> - ...?

No, skip is saying that the tests cannot be run as the environment
does not support the functionality we require. Do you fail the Win9x
tests just because they don't support *W API variants? Or Windows
environments that don't have DirectX 7, 8, 9 or 10 installed?

>>Shouldn't this be something like:
>>      if(err == MCIERR_INVALID_FILE) {
>>          skip("tempfile.wav was not found (not saved?), skipping\n");
> No. If any error happens in open, we can't continue. Perhaps you mean
> if (err) {
>   ok(err==MCIERR_FILE_NOT_FOUND, "...\n"); /* the expected situation */
>   skip("tempfile.wav was not found (not saved?), skipping\n");

I meant MCIERR_FILE_NOT_FOUND, but the point was not to error in this
case. Therefore, something like this:

   err=mciSendString("open ....");
   switch (err)
   {
   case MCIERR_FILE_NOT_FOUND:
      skip("tempfile.wav was not found (not saved?)");
      return;
   case MCIERR_CANNOT_LOAD_DRIVER:
   case MCIERR_...:
      skip("unable to open the mci sound driver (%s) - is sound
enabled?", dbg_mcierr(err));
      return;
   default:
      ok(!err, "mciSendString('open') failed, expected NOERROR, got
%s", dbg_mcierr(err));
   }

But then, as you said, any failure here means that you cannot proceed.
However, the main cases are:
   1.  tempfile.wav is not present;
   2.  sound is not present on the system.

The other errors (MCIERR_UNRECOGNISED_COMMAND, MCIERR_HARDWARE,
MCIERR_OUT_OF_MEMORY) are either:
   1.  situations that cannot occur in the given test;
   2.  are unexpected error cases (i.e. situations that we do not
expect on a normal run).

Therefore, the above code (with the correct error values) should be
sufficient. If we find that other situations need correcting (due to
failing tests), they can be looked at then.

- Reece

[Joerg-Cyril.Hoehle]

Reece,

I think we reached agreement on:
a) Tests should "usually" succeed.
b) machines without sound are usual (e.g. in virtual server env.).
c) I'll modify the MCI tests to succeed on machines without sound.
But for some other points, I've trouble following you.


>> The relevant question becomes: when is a configuration unusual? When
>> can it be ignored for the purpose of Wine and when is it relevant?
>Answer: no configuration is unusual.
Huh?
>The point is, if the machine has a broken soundcard (or in Wine does
>not have an available sound driver, or is broken due to PulseAudio on
>Ubuntu), the tests should still pass (or be skipped) as is
>appropriate.

This seems wrong (in general) to me, even though Paul somehow
expresses an opinion similar to yours.  I say you cannot write a
program meant to run in a completely unknown environment, doing
comprehensive tests and not stumbling upon strange behaviour.
The program is specifically designed to detect and flag deviant behaviour.
All that can be achieved in a program is to be prepared for a known
set of scenarios (a broken soundcard is such a scenerio), aka. use-case.
Outside these known use-cases, the tests throw an error.  Otherwise
there's no point in ok() at all.

I agree that for the known use-cases (these explicitly include
configurations without sound), the tests should succeed.

What can be done is what you say:
>If we find that other situations need correcting (due to
>failing tests), they can be looked at then.
... then decide whether the cause of the failing test is "unusual" or not.


I feel that I shall stop discussing that issue.  If the consensus
is good enough, we need not take this further to eventually discover
that the sole disagreement is on the exact meaning of some English words.


>It also seems to me that there should be some tests that test opening
>an mci stream on a file that we know not to exist (e.g.
>this-file-does-not-exist.wav) so that those failure cases can be
>properly documented.
Good idea, esp. that w95 returns different error codes for foo.wav and bar\foo.wav!


>The point I am making is that:
>  1.  The error and skip statements are saying the same thing;
>  2.  But the skip message is misleading.
Indeed, sorry for that.
There's a dependency.  The tests depend on record working to generate the
file to be played, instead of including one in git.
So they fail to test machines that can play but not record.
Obviously I'll have to do something else in the yet to be written MIDI tests.

BTW, this raises the question whether Wine should include a tiny wave file
for completeness.  Invoking the MCI Sound command always plays some sound.


>If I have counted the offsets correctly 275 is
>MCIERR_DEVICE_NOT_READY, *not* that tempfile.wav was not found.
Wrong.  275 - 256 is 19, which you look up in mmsystem.h:
#define MCIERR_FILE_NOT_FOUND   (MCIERR_BASE + 19)


>   default:
>      ok(!err, "mciSendString('open') failed, expected NOERROR, got
>%s", dbg_mcierr(err));
>   }
In the default case, if (err) you must return (with or without skip message).
You cannot continue if open fails.  So it summarizes to:
if (err) {
    /* list of situations that were analysed as "usual": */
    ok(err=FILE_NOT_FOUND || err==MCIERR_CANNOT_LOAD_DRIVER || err==xyz ...);
   skip("Cannot test playback, cause %s",dbg_mcierr(err));
   return
}

>   case MCIERR_CANNOT_LOAD_DRIVER:
>   2.  sound is not present on the system.
Perhaps there's a misconception? That's not how MS-Windows systems without
sound drivers present themselves.  Ge's, Paul's and my father in law's
machines all include the MCI (and waveaudio.dll etc.).  It's only at
WAVE_Open time that failure is detected, e.g. Cue, Record or Play, and
the error is MCIERR_WAVE_INPUTSUNSUITABLE for instance.
(That's not entirely true, it does not explain the setformat pcm failure).
I.e. no mciwave present at all so far is an "unusual" situation, and generates an error.

Regards,
        Jörg Höhle.

[Paul Vriens-2]

On 11/09/2009 07:25 PM, Joerg-Cyril.Hoehle@... wrote:
> Reece,
>
> I think we reached agreement on:
> a) Tests should "usually" succeed.

I didn't read that in any of the emails. Tests should always succeed.
the broken() and skip() will help in achieving that.

>> The point is, if the machine has a broken soundcard (or in Wine does
>> not have an available sound driver, or is broken due to PulseAudio on
>> Ubuntu), the tests should still pass (or be skipped) as is
>> appropriate.
>
> This seems wrong (in general) to me, even though Paul somehow
> expresses an opinion similar to yours.  I say you cannot write a
> program meant to run in a completely unknown environment, doing
> comprehensive tests and not stumbling upon strange behaviour.

I'm now busy for example with the eventlog stuff. I create loads of
tests and run them on my pretty clean boxes (W95 up to Win7). Only when
they pass on all boxes I sent the patches. If test.winehq.org shows
failures for these new tests they have to be dealt with.

I agree that not everybody has this multitude of test boxes but there
are always people around willing to test new tests on their Windows
boxes (real and/or virtual).

> Regards,
> Jörg Höhle.
>
>

--
Cheers,

Paul.

[Reece Dunn-2]

2009/11/9 Paul Vriens <paul.vriens.wine@...>:
> On 11/09/2009 07:25 PM, Joerg-Cyril.Hoehle@... wrote:
>>
>> Reece,
>>
>> I think we reached agreement on:
>> a) Tests should "usually" succeed.
>
> I didn't read that in any of the emails. Tests should always succeed. the
> broken() and skip() will help in achieving that.

I used "usually" succeed to avoid handling out-of-memory cases and
other similar cases.

- Reece

[Reece Dunn-2]

2009/11/9  <Joerg-Cyril.Hoehle@...>:
> Reece,
>
> I think we reached agreement on:
> a) Tests should "usually" succeed.
> b) machines without sound are usual (e.g. in virtual server env.).
> c) I'll modify the MCI tests to succeed on machines without sound.
> But for some other points, I've trouble following you.

Yes.

>>> The relevant question becomes: when is a configuration unusual? When
>>> can it be ignored for the purpose of Wine and when is it relevant?
>>Answer: no configuration is unusual.
> Huh?

You asked "when is a configuration unusual?" That is, are the following unusual:
  1/  using a non-English locale (e.g. German or Polish);
  2/  using a non-Western locale (e.g. Chinese or Japanese);
  3/  using classic theme in XP or later;
  4/  using 120DPI or higher;
  5/  using a high-contrast theme;
  6/  not having DirectX installed;
  7/  having a Windows install on a drive other than C:\
  8/  ...

My point is that none of these (and other configurations/setups in
Windows) should be considered unusual. They may (or may not) be the
default, but they are possible.

From the perspective of Wine (and the Wine tests), if it is possible
in Windows (on VMware or natively) then it should be handled properly
in the tests and not fail the tests.

>>The point is, if the machine has a broken soundcard (or in Wine does
>>not have an available sound driver, or is broken due to PulseAudio on
>>Ubuntu), the tests should still pass (or be skipped) as is
>>appropriate.
>
> This seems wrong (in general) to me, even though Paul somehow
> expresses an opinion similar to yours.  I say you cannot write a
> program meant to run in a completely unknown environment, doing
> comprehensive tests and not stumbling upon strange behaviour.

Write the tests initially for the known valid environments, then fix
up issues as they are seen on test.winehq.org (for example, if some of
the comctl32/user32 tests fail because they are run in 120DPI instead
of 96DPI).

You cannot cater for a completely unknown environment, but those on
test.winehq.org are not unknown.

Also, it is not feasible to always do comprehensive tests (e.g. on
Win9x when *W functions are not present). The point is to do what you
can in those cases.

> The program is specifically designed to detect and flag deviant behaviour.
> All that can be achieved in a program is to be prepared for a known
> set of scenarios (a broken soundcard is such a scenerio), aka. use-case.
> Outside these known use-cases, the tests throw an error.  Otherwise
> there's no point in ok() at all.

You have a set of defined behaviour for the mci API when there is a
sound card present. Good -- the tests should cover that.

You also have a set of defined behaviour for the mci API when a sound
card/driver is not present. Good -- the tests should handle this (e.g.
by defining the set of valid return values).

> I agree that for the known use-cases (these explicitly include
> configurations without sound), the tests should succeed.

Yes.

> What can be done is what you say:
>>If we find that other situations need correcting (due to
>>failing tests), they can be looked at then.
> ... then decide whether the cause of the failing test is "unusual" or not.

No -- look at *why* the test is failing. This will either be that the
test is doing something wrong (e.g. not creating a test file), or that
it is not covering a valid situation.

The cause being "unusual" or not has no relevance. Either the test is
wrong, or Windows is broken.

>>The point I am making is that:
>>  1.  The error and skip statements are saying the same thing;
>>  2.  But the skip message is misleading.
> Indeed, sorry for that.
> There's a dependency.  The tests depend on record working to generate the
> file to be played, instead of including one in git.
> So they fail to test machines that can play but not record.

Sure.

> Obviously I'll have to do something else in the yet to be written MIDI tests.
>
> BTW, this raises the question whether Wine should include a tiny wave file
> for completeness.  Invoking the MCI Sound command always plays some sound.

Is it possible to generate the wav file programatically, without using
the record functionality?

>>If I have counted the offsets correctly 275 is
>>MCIERR_DEVICE_NOT_READY, *not* that tempfile.wav was not found.
> Wrong.  275 - 256 is 19, which you look up in mmsystem.h:
> #define MCIERR_FILE_NOT_FOUND   (MCIERR_BASE + 19)

Ah yes.

>>   default:
>>      ok(!err, "mciSendString('open') failed, expected NOERROR, got
>>%s", dbg_mcierr(err));
>>   }
> In the default case, if (err) you must return (with or without skip message).
> You cannot continue if open fails.  So it summarizes to:
> if (err) {
>    /* list of situations that were analysed as "usual": */
>    ok(err=FILE_NOT_FOUND || err==MCIERR_CANNOT_LOAD_DRIVER || err==xyz ...);
>   skip("Cannot test playback, cause %s",dbg_mcierr(err));
>   return
> }
>
>>   case MCIERR_CANNOT_LOAD_DRIVER:
>>   2.  sound is not present on the system.
> Perhaps there's a misconception? That's not how MS-Windows systems without
> sound drivers present themselves.  Ge's, Paul's and my father in law's
> machines all include the MCI (and waveaudio.dll etc.).  It's only at
> WAVE_Open time that failure is detected, e.g. Cue, Record or Play, and
> the error is MCIERR_WAVE_INPUTSUNSUITABLE for instance.
> (That's not entirely true, it does not explain the setformat pcm failure).
> I.e. no mciwave present at all so far is an "unusual" situation, and generates an error.

In the example I gave, don't look at the exact error codes -- look
more at the situations (tempfile.wav does not exist, sound not
available).

Also,

 if (err==MCIERR_FILE_NOT_FOUND || err==...) {
   skip("Cannot test playback (err=%s)",dbg_mcierr(err));
   return;
 }
 ok(!err, ...);

The point here is that tempfile.wav not existing, sound not available,
or a broken sound driver should not cause the ok line to fail. These
are valid situations that mean that the playback tests cannot be run.

Now, arguably the tests should be modified so that tempfile.wav always
exist. This would then address a chunk of the test failures that are
happening on all machines at present.

IIUC, that file is being generated through the record API of mci?
Isn't there a way to programatically create the WAV/PCM data through
the use of math.h functions?

- Reece