[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