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

James Hawkins truiken at gmail.com
Mon Jun 11 03:05:24 CDT 2007


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':

+            skip("InstallHinfSectionW is broken, using InstallHinfSectionA\n");

+            useW = 0;

+            _InstallHinfSection(NULL, NULL, cmdline, 0);

-- 
James Hawkins



More information about the wine-devel mailing list