Re: advapi32/tests: Test for last error after RegQueryValueEx
Nikolay Sivov <nsivov(a)codeweavers.com> writes:
@@ -626,15 +627,22 @@ static void test_query_value_ex(void) trace("test_query_value_ex: type set to: 0x%08x\n", type); ok(size == 0, "size should have been set to 0 instead of %d\n", size); } + todo_wine ok(GetLastError() == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %d\n", GetLastError());
size = sizeof(buffer); + /* this last error check could be removed after setting it properly is fixed, + it's to prevent regressions from current state */ + SetLastError(ERROR_SUCCESS); ret = RegQueryValueExA(HKEY_CLASSES_ROOT, "Nonexistent Value", NULL, &type, buffer, &size); ok(ret == ERROR_FILE_NOT_FOUND, "expected ERROR_FILE_NOT_FOUND, got %d\n", ret); ok(size == sizeof(buffer), "size shouldn't have been changed to %d\n", size); + ok(GetLastError() == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %d\n", GetLastError());
This is confusing. You should set it to 0xdeadbeef before each call for the check to be meaningful. -- Alexandre Julliard julliard(a)winehq.org
On 2/26/2010 16:38, Alexandre Julliard wrote:
Nikolay Sivov<nsivov(a)codeweavers.com> writes:
@@ -626,15 +627,22 @@ static void test_query_value_ex(void) trace("test_query_value_ex: type set to: 0x%08x\n", type); ok(size == 0, "size should have been set to 0 instead of %d\n", size); } + todo_wine ok(GetLastError() == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %d\n", GetLastError());
size = sizeof(buffer); + /* this last error check could be removed after setting it properly is fixed, + it's to prevent regressions from current state */ + SetLastError(ERROR_SUCCESS); ret = RegQueryValueExA(HKEY_CLASSES_ROOT, "Nonexistent Value", NULL,&type, buffer,&size); ok(ret == ERROR_FILE_NOT_FOUND, "expected ERROR_FILE_NOT_FOUND, got %d\n", ret); ok(size == sizeof(buffer), "size shouldn't have been changed to %d\n", size); + ok(GetLastError() == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %d\n", GetLastError());
This is confusing. You should set it to 0xdeadbeef before each call for the check to be meaningful.
Ok, I'll remove this part. Reason was to point a case when check is working with current code.
Nikolay Sivov <bunglehead(a)gmail.com> writes:
On 2/26/2010 16:38, Alexandre Julliard wrote:
Nikolay Sivov<nsivov(a)codeweavers.com> writes:
@@ -626,15 +627,22 @@ static void test_query_value_ex(void) trace("test_query_value_ex: type set to: 0x%08x\n", type); ok(size == 0, "size should have been set to 0 instead of %d\n", size); } + todo_wine ok(GetLastError() == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %d\n", GetLastError());
size = sizeof(buffer); + /* this last error check could be removed after setting it properly is fixed, + it's to prevent regressions from current state */ + SetLastError(ERROR_SUCCESS); ret = RegQueryValueExA(HKEY_CLASSES_ROOT, "Nonexistent Value", NULL,&type, buffer,&size); ok(ret == ERROR_FILE_NOT_FOUND, "expected ERROR_FILE_NOT_FOUND, got %d\n", ret); ok(size == sizeof(buffer), "size shouldn't have been changed to %d\n", size); + ok(GetLastError() == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %d\n", GetLastError());
This is confusing. You should set it to 0xdeadbeef before each call for the check to be meaningful.
Ok, I'll remove this part. Reason was to point a case when check is working with current code.
I'm not saying that you should remove it, but that you should set last error to 0xdeadbeef to make it clear whether you are expecting it to be unchanged or set to 0. Right now your test could mean either. -- Alexandre Julliard julliard(a)winehq.org
On 2/26/2010 19:00, Alexandre Julliard wrote:
Nikolay Sivov<bunglehead(a)gmail.com> writes:
On 2/26/2010 16:38, Alexandre Julliard wrote:
Nikolay Sivov<nsivov(a)codeweavers.com> writes:
@@ -626,15 +627,22 @@ static void test_query_value_ex(void) trace("test_query_value_ex: type set to: 0x%08x\n", type); ok(size == 0, "size should have been set to 0 instead of %d\n", size); } + todo_wine ok(GetLastError() == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %d\n", GetLastError());
size = sizeof(buffer); + /* this last error check could be removed after setting it properly is fixed, + it's to prevent regressions from current state */ + SetLastError(ERROR_SUCCESS); ret = RegQueryValueExA(HKEY_CLASSES_ROOT, "Nonexistent Value", NULL,&type, buffer,&size); ok(ret == ERROR_FILE_NOT_FOUND, "expected ERROR_FILE_NOT_FOUND, got %d\n", ret); ok(size == sizeof(buffer), "size shouldn't have been changed to %d\n", size); + ok(GetLastError() == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %d\n", GetLastError());
This is confusing. You should set it to 0xdeadbeef before each call for the check to be meaningful.
Ok, I'll remove this part. Reason was to point a case when check is working with current code.
I'm not saying that you should remove it, but that you should set last error to 0xdeadbeef to make it clear whether you are expecting it to be unchanged or set to 0. Right now your test could mean either.
Previous check is about the same thing, I reset to deadbeef and check for 0 after call. Second one is what application does, I wanted it as a passing test to show that error isn't changed from 0. It's ok to remove it.
Nikolay Sivov <bunglehead(a)gmail.com> writes:
On 2/26/2010 19:00, Alexandre Julliard wrote:
I'm not saying that you should remove it, but that you should set last error to 0xdeadbeef to make it clear whether you are expecting it to be unchanged or set to 0. Right now your test could mean either.
Previous check is about the same thing, I reset to deadbeef and check for 0 after call. Second one is what application does, I wanted it as a passing test to show that error isn't changed from 0. It's ok to remove it.
The second one doesn't behave the same as the first one for me. Also setting last error at all here is surprising, so I think you'll want all the tests you can have. -- Alexandre Julliard julliard(a)winehq.org
On 2/26/2010 19:17, Alexandre Julliard wrote:
Nikolay Sivov<bunglehead(a)gmail.com> writes:
On 2/26/2010 19:00, Alexandre Julliard wrote:
I'm not saying that you should remove it, but that you should set last error to 0xdeadbeef to make it clear whether you are expecting it to be unchanged or set to 0. Right now your test could mean either.
Previous check is about the same thing, I reset to deadbeef and check for 0 after call. Second one is what application does, I wanted it as a passing test to show that error isn't changed from 0. It's ok to remove it.
The second one doesn't behave the same as the first one for me. Also setting last error at all here is surprising, so I think you'll want all the tests you can have.
Strange. Ok, will look closer.
participants (2)
-
Alexandre Julliard -
Nikolay Sivov