"Detlef Riekenberg" <wine.dev(a)web.de> wrote:
> @@ -939,36 +939,36 @@ static void test_GetPrinterDriver(void)
>
> for (level = -1; level <= 7; level++)
> {
> - SetLastError(0xdeadbeef);
> + SetLastError(MAGIC_DEAD);
Please don't change this, stick to the style used everywhere else, i.e. use
0xdeadbeef directly.
> needed = (DWORD)-1;
> ret = GetPrinterDriver(hprn, NULL, level, NULL, 0, &needed);
> - ok(!ret, "level %d: GetPrinterDriver should fail\n", level);
> - if (level >= 1 && level <= 6)
> - {
> - ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "wrong error %ld\n", GetLastError());
> - ok(needed > 0,"not expected needed buffer size %ld\n", needed);
> - }
> - else
> - {
> - ok(GetLastError() == ERROR_INVALID_LEVEL, "wrong error %ld\n", GetLastError());
> - ok(needed == (DWORD)-1,"not expected needed buffer size %ld\n", needed);
> - continue;
> - }
> + ok(!ret && ((GetLastError() == ERROR_INVALID_LEVEL) ||
> + (GetLastError() == ERROR_INSUFFICIENT_BUFFER) ||
> + (GetLastError() == ERROR_OUTOFMEMORY)), "%d: returned %d with %ld " \
> + "(expected '0' with: ERROR_INVALID_LEVEL, ERROR_INSUFFICIENT_BUFFER" \
> + "or ERROR_OUTOFMEMORY)\n", level, ret, GetLastError());
> +
> + if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) continue;
> +
> + ok(needed > 0,"%d: returned size %ld\n", level, needed);
> + if (needed == 0) continue;
I object to this change. The test needs a clear indication whether it's an invalid
or a not supported parameter. Do not remove if {} else {}, that's an essential part
of the test. A comment explaining the change would be also nice.
> ret = GetPrinterDriver(hprn, NULL, level, buf, needed, &filled);
> - ok(ret, "level %d: GetPrinterDriver error %ld\n", level, GetLastError());
> - ok(needed == filled, "needed %ld != filled %ld\n", needed, filled);
> + ok( ret, "%d: returned %d with %ld (expected '!= 0')\n",
> + level, ret, GetLastError());
"expected '!= 0'" is wrong, ret is BOOL, so "expected TRUE" or better leave it as
it is now, error message already implies that the API has failed.
> - ok(di_2->cVersion >= 0 && di_2->cVersion <= 3, "di_2->cVersion = %ld\n", di_2->cVersion);
> + ok((di_2->cVersion >= 0 && di_2->cVersion <= 3) ||
> + (di_2->cVersion == 0x0400), "di_2->cVersion = %ld\n", di_2->cVersion);
A comment explaining which windows version returns 0x400 (which contradicts MSDN)
would be helpful.
> ok(di_2->pName != NULL, "not expected NULL ptr\n");
> ok(di_2->pEnvironment != NULL, "not expected NULL ptr\n");
> ok(di_2->pDriverPath != NULL, "not expected NULL ptr\n");
> @@ -977,15 +977,15 @@ static void test_GetPrinterDriver(void)
>
> trace("cVersion %ld\n", di_2->cVersion);
> trace("pName %s\n", di_2->pName);
> - calculated += strlen(di_2->pName) + 1;
> + calculated += lstrlenA(di_2->pName) + 1;
If you want to replace strlen by lstrlenA (why?) that should be a separate
patch. Same for below changes.
--
Dmitry.