How do I actually write a testcase for a 16-bit API ?

[Alex Villací­s Lasso]

I recently filed http://bugs.winehq.org/show_bug.cgi?id=18734 in which I 
showed that DlgDirList is behaving incorrectly for 16-bit apps. I also 
included a test program compiled with the Watcom C compiler, as well as 
a patch. Then a comment appeared saying that it should be possible to 
create a testcase for this API. So I tried. At first, I thought that 
since this was a 16-bit API, I should be adding -Wb,--subsystem,win16 to 
the Makefile. However, this results in the linker complaining that it 
cannot find WinMain16. So I removed it. Also, I had undefined link-time 
references to SendMessage16 and similar (because I want everything to go 
through the 16-bit APIs, else the whole point of the test is lost), so I 
had to manually add them to the spec files for kernel32 and user32. But 
after doing this and getting a test program, an attempt to create the 
window out of the registered class fails and triggers an assertion.

I have no examples of previous 16-bit tests (as opposed to 16-bit Wine 
dlls) to draw upon, so I don't know how to proceed correctly to test a 
16-bit API in a way that convinces everyone that the patch is correct. 
What do you think of this?

[Austin English]

On Mon, Jun 8, 2009 at 2:46 PM, Alex Villací­s
Lasso wrote:
> I recently filed http://bugs.winehq.org/show_bug.cgi?id=18734 in which I
> showed that DlgDirList is behaving incorrectly for 16-bit apps. I also
> included a test program compiled with the Watcom C compiler, as well as a
> patch. Then a comment appeared saying that it should be possible to create a
> testcase for this API. So I tried. At first, I thought that since this was a
> 16-bit API, I should be adding -Wb,--subsystem,win16 to the Makefile.
> However, this results in the linker complaining that it cannot find
> WinMain16. So I removed it. Also, I had undefined link-time references to
> SendMessage16 and similar (because I want everything to go through the
> 16-bit APIs, else the whole point of the test is lost), so I had to manually
> add them to the spec files for kernel32 and user32. But after doing this and
> getting a test program, an attempt to create the window out of the
> registered class fails and triggers an assertion.
>
> I have no examples of previous 16-bit tests (as opposed to 16-bit Wine dlls)
> to draw upon, so I don't know how to proceed correctly to test a 16-bit API
> in a way that convinces everyone that the patch is correct. What do you
> think of this?

http://www.winehq.org/pipermail/wine-devel/2007-July/058084.html
http://www.winehq.org/pipermail/wine-devel/2007-July/057917.html
http://win16test.googlecode.com/

[Alex Villací­s Lasso]

Austin English escribió:
> On Mon, Jun 8, 2009 at 2:46 PM, Alex Villací­s
> Lasso wrote:
> > I recently filed http://bugs.winehq.org/show_bug.cgi?id=18734 in which I
> > showed that DlgDirList is behaving incorrectly for 16-bit apps. I also
> > included a test program compiled with the Watcom C compiler, as well as a
> > patch. Then a comment appeared saying that it should be possible to create a
> > testcase for this API. So I tried. At first, I thought that since this was a
> > 16-bit API, I should be adding -Wb,--subsystem,win16 to the Makefile.
> > However, this results in the linker complaining that it cannot find
> > WinMain16. So I removed it. Also, I had undefined link-time references to
> > SendMessage16 and similar (because I want everything to go through the
> > 16-bit APIs, else the whole point of the test is lost), so I had to manually
> > add them to the spec files for kernel32 and user32. But after doing this and
> > getting a test program, an attempt to create the window out of the
> > registered class fails and triggers an assertion.
> >
> > I have no examples of previous 16-bit tests (as opposed to 16-bit Wine dlls)
> > to draw upon, so I don't know how to proceed correctly to test a 16-bit API
> > in a way that convinces everyone that the patch is correct. What do you
> > think of this?
>
> http://www.winehq.org/pipermail/wine-devel/2007-July/058084.html
> http://www.winehq.org/pipermail/wine-devel/2007-July/057917.html
> http://win16test.googlecode.com/

I downloaded the tests from  win16test.googlecode.com , but the tarball 
already has a small backlog of patches that have not yet been integrated 
into Wine (checked with patch --dry-run). Why? I could make a patch to 
the testsuite from win16test.googlecode.com, but will this help at all 
on getting the fix integrated into Wine?

[Steven Edwards]

On Mon, Jun 8, 2009 at 4:09 PM, Alex Villací­s
Lasso wrote:
> I downloaded the tests from  win16test.googlecode.com , but the tarball
> already has a small backlog of patches that have not yet been integrated
> into Wine (checked with patch --dry-run). Why? I could make a patch to the
> testsuite from win16test.googlecode.com, but will this help at all on
> getting the fix integrated into Wine?

We need someone to do the configure magic of detecting OpenWatcom if
installed and teach configure and friends how to build. The win16 test
suite has kind of been in bitrot mode as no one has taken the time to
integrate it. Of course you might want to double check with Alexandre
as to his preference. He might prefer at this point to just keep the
win16 tests out of the tree. I don't know.

[Alexandre Julliard]

Steven Edwards  writes:
> On Mon, Jun 8, 2009 at 4:09 PM, Alex Villací­s
> Lasso wrote:
> > I downloaded the tests from  win16test.googlecode.com , but the tarball
> > already has a small backlog of patches that have not yet been integrated
> > into Wine (checked with patch --dry-run). Why? I could make a patch to the
> > testsuite from win16test.googlecode.com, but will this help at all on
> > getting the fix integrated into Wine?
>
> We need someone to do the configure magic of detecting OpenWatcom if
> installed and teach configure and friends how to build. The win16 test
> suite has kind of been in bitrot mode as no one has taken the time to
> integrate it. Of course you might want to double check with Alexandre
> as to his preference. He might prefer at this point to just keep the
> win16 tests out of the tree. I don't know.

They can remain out of tree, but still, being able to demonstrate a
working test case will help getting the patch in, even if the test
remains out for now.

[Dmitry Timoshkov]

"Alex Villací­s Lasso"  wrote:
> I recently filed http://bugs.winehq.org/show_bug.cgi?id=18734 in which I showed that DlgDirList is behaving 
> incorrectly for 16-bit apps. I also included a test program compiled with the Watcom C compiler, as well as a patch. 
> Then a comment appeared saying that it should be possible to create a testcase for this API.

By creating a test I meant to have a test for 32-bit behaviour first,
then compare it with 16-bit one.

[Alex Villací­s Lasso]

Dmitry Timoshkov escribió:
> "Alex Villací­s Lasso"  wrote:
> > I recently filed http://bugs.winehq.org/show_bug.cgi?id=18734 in 
> > which I showed that DlgDirList is behaving incorrectly for 16-bit 
> > apps. I also included a test program compiled with the Watcom C 
> > compiler, as well as a patch. Then a comment appeared saying that it 
> > should be possible to create a testcase for this API.
>
> By creating a test I meant to have a test for 32-bit behaviour first,
> then compare it with 16-bit one.

The 32-bit test is already in place in current git, and shows that the 
behavior is compliant for 32-bit apps. It is at 
dlls/user32/tests/listbox.c. Lines 1189 through 1219 show that 
DDL_DRIVES alone implies DDL_EXCLUSIVE. Lines 1231 through 1263 show 
that DDL_DRIVES|DDL_DIRECTORY does *not* imply DDL_EXCLUSIVE.

I am taking issue with lines 617 through 619 of dlls/user32/dialog16.c:
----------------
/**************************************************************************
 *              DlgDirList   (USER.100)
 */
INT16 WINAPI DlgDirList16( HWND16 hDlg, LPSTR spec, INT16 idLBox,
                           INT16 idStatic, UINT16 attrib )
{
    /* according to Win16 docs, DDL_DRIVES should make DDL_EXCLUSIVE
     * be set automatically (this is different in Win32, and
     * DIALOG_DlgDirList sends Win32 messages to the control,
     * so do it here) */
    if (attrib & DDL_DRIVES) attrib |= DDL_EXCLUSIVE;
    return DlgDirListA( WIN_Handle32(hDlg), spec, idLBox, idStatic, 
attrib );
}
----------------

If a 16-bit test were placed in the testsuite, it would show that the 
current implementation (attrib & DDL_DRIVES) is incorrect, as 
demonstrated by my test program. It should be attrib == DDL_DRIVES 
instead. The program so far shows that the flag behavior is identical 
for 16-bit and 32-bit calls.