On 6/10/07, Misha Koshelev mk144210@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);