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

Misha Koshelev mk144210 at bcm.edu
Mon Jun 11 07:56:46 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.
> 
> +        _InstallHinfSection(NULL, NULL, cmdline, 0);
> 
> Why aren't you checking the return value, or GetLastError?
> 
> +            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.
> 
> +        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':
I meant START_TEST(install) is where this is taken care of, not
TEST(install).

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

Misha



More information about the wine-devel mailing list