[PATCH 1/3] setupapi/tests: Add rudimentary tests for InstallHinfSectionA/W.

Misha Koshelev mk144210 at bcm.edu
Mon Jun 11 07:51:54 CDT 2007


On Mon, 2007-06-11 at 01:05 -0700, James Hawkins wrote:
> On 6/10/07, Misha Koshelev <mk144210 at bcm.edu> wrote:
> > Tested on WinXP and Win98 (and of course wine).
> > ---
> >  dlls/setupapi/tests/Makefile.in |    1 +
> >  dlls/setupapi/tests/install.c   |  152 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 153 insertions(+), 0 deletions(-)
> >
> 
> +static void test_install(LPCSTR filename)
> 
> +{
> 
> +    char cmdline[MAX_PATH * 2];
> 
> +    LONG ret;
> 
> +
> 
> +    sprintf(cmdline, "DefaultInstall 128 %s\\%s", CURR_DIR, filename);
> 
> +
> 
> +    if (useW == -1)
> 
> +    {
> 
> +        HKEY hkey;
> 
> +        useW = 1;
> 
> +        _InstallHinfSection(NULL, NULL, cmdline, 0);
> 
> +        if (RegOpenKey(HKEY_CURRENT_USER,
> "Software\\Wine\\setupapitest", &hkey) == ERROR_SUCCESS)
> 
> +            RegCloseKey(hkey);
> 
> +        else
> 
> +        {
> 
> +            skip("InstallHinfSectionW is broken, using InstallHinfSectionA\n");
> 
> +            useW = 0;
> 
> +            _InstallHinfSection(NULL, NULL, cmdline, 0);
> 
> +        }
> 
> +    }
> 
> +    else _InstallHinfSection(NULL, NULL, cmdline, 0);
> 
> +
> 
> +    ret = RegDeleteKey(HKEY_CURRENT_USER, "Software\\Wine\\setupapitest");
> 
> +    ok(ret == ERROR_SUCCESS, "Expected registry key
> Software\\Wine\\setupapitest to exist, RegDeleteKeyA returned %d\n",
> ret);
> 
> +}
> 
> +
> 
> +static void test_InstallHinfSection(void)
> 
> +{
> 
> +    create_inf_file(inffile);
> 
> +    test_install(inffile);
> 
> +    ok(DeleteFile(inffile), "Expected source inf to exist, last error
> was %d\n", GetLastError());
> 
> +}
> 
> This would be much simpler if you didn't use the extra test_install
> function.  What is the point of that?  Unit test functions should be
> atomic.  Each test function should create the files it needs, and make
> sure the files it created are cleaned up by the end of the function
> (replace files with registry keys, etc).  You can also look at it like
> this: you should be able to remove test function x without affecting
> any other tests, which is not the case here.  The check for DeleteFile
> would obviously fail.  Unit tests should be clear and easy to
> understand what's being tested, and honestly, this code confused me on
> my first reading.
I will rename test_install, I think the test_ naming of it is confusing.
It is just another helper function, and the point is more clear in patch
2 I think (where I use a space in the path and show that it doesn't work
on our implementation).

> 
> +        _InstallHinfSection(NULL, NULL, cmdline, 0);
> 
> Why aren't you checking the return value, or GetLastError?
Both are void functions. I can check for GetLastError() actually working
for this function but it is not documented in msdn to do so.

> 
> +            skip("InstallHinfSectionW is broken, using InstallHinfSectionA\n");
> 
> How is it broken?  This needs to be clarified with a comment.  You
> really should just test only the A function in the first place though.
Yes, that is what I tried first just using the A function. However, on
Windows XP the functionality of this function is that it returns and
does not do anything. I thought something was wrong with my code at
first, but the inf file being generated was actually installed just fine
from the command line, and I found a comment on a Russian language
programming forum with someone who had the same problem and just
recommended switching to the W function. I switched to W and it worked
fine with no other changes.

Similary, on Win95, the W function exists but seems to have this
functionality of not doing anything ,whereas the A function works
perfectly with the exact same inf and cmdline.

I will add a more detailed comment about this.

> 
> +        if (pInstallHinfSectionW && !pInstallHinfSectionA) useW = 1;
> 
> Is there ever a time when InstallHinfSectionA doesn't exist?  If it
> doesn't exist, you're going to crash if InstallHinfSectionW is
> 'broken':
This case is taken care of in the TEST(install) section (actually all
cases of functions not existing). useW only stays -1 if both functions
exist.

> 
> +            skip("InstallHinfSectionW is broken, using InstallHinfSectionA\n");
> 
> +            useW = 0;
> 
> +            _InstallHinfSection(NULL, NULL, cmdline, 0);
> 

Misha



More information about the wine-devel mailing list