[PATCH 1/2] kernel32/tests: Add UTF-7 non-encoded surrounding characters tests.

[Sebastian Lackner]

On 05.11.2014 15:38, Alexandre Julliard wrote:
> Alex Henrie <alexhenrie24 at gmail.com> writes:
> 
>> +        SetLastError(0xdeadbeef);
>> +        len = WideCharToMultiByte(CP_UTF7, 0, input, 4, output, sizeof(output) - 1, NULL, NULL);
>> +        ok(GetLastError() == 0xdeadbeef,
>> +           "i=0x%04x: expected error=0xdeadbeef, got error=0x%x\n", i, GetLastError());
> 
> Testing last error on success is not useful and should be avoided.
> 

Whats the reason for this Wine decision? Its not a very useful one at my opinion.

Besides the fact that a lot of existing tests check that GetLastError() was unmodified (20 matches for "== 0xdeadbeef" in kernel32/tests/codepage.c), its important to know the exact behaviour when trying to propagate the error code, similar to how HeapFree(...) should keep the original error code on success.

The fact that there are also API functions where the error code is indeed modified on success (for example CreateMutexExW) makes me think that such a test can basically never hurt, to avoid hunting down weird failures in the future. It would not be the first example where programs check for success the wrong way, and only look at the error code.

Even if native would behave differently, and we probably would decide not to reproduce the behaviour - the tests are a good documentation of how it is supposed to work. The next person who wants to know if WideCharToMultibyte(CP_UTF7, ...) changes the error code, will have to write those tests just again.

Any good reason behind this?

Regards,
Sebastian