[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: [PATCH 1/4] kernel32/tests: Add simple UTF-7 encoding tests.
From: Sebastian Lackner <sebastian () fds-team ! de>
Date: 2014-10-29 20:38:39
Message-ID: 5451504F.4040206 () fds-team ! de
[Download RAW message or body]
On 20.10.2014 06:41, Alex Henrie wrote:
> ---
> dlls/kernel32/tests/codepage.c | 134 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 134 insertions(+)
>
> diff --git a/dlls/kernel32/tests/codepage.c b/dlls/kernel32/tests/codepage.c
> index 8423c75..684ede1 100644
> --- a/dlls/kernel32/tests/codepage.c
> +++ b/dlls/kernel32/tests/codepage.c
> @@ -412,6 +412,138 @@ static void test_string_conversion(LPBOOL bUsedDefaultChar)
> ok(GetLastError() == 0xdeadbeef, "GetLastError() is %u\n", GetLastError());
> }
>
> +static void test_utf7_encoding(void)
> +{
> + struct simple_test
> + {
> + WCHAR utf16[1024];
> + int utf16_len;
> + char utf7[1024];
> + int utf7_len;
1024 is unnecessary big. A couple of bytes would be sufficient for all these tests.
> + };
> +
> + static const struct simple_test simple_tests[] = {
No need to give that struct a name. You can simply do:
static const struct
{
...
}
simple_tests[] =
{
...
}
> + /* tests some valid UTF-16 */
> + {
> + {0x4F60,0x597D,0x5417,0},
> + 4,
> + "+T2BZfVQX-",
> + 11
> + },
> + /* tests some invalid UTF-16 */
> + /* (stray lead surrogate) */
That looks ugly, either put it in a single line or use:
/* line1
* line2 */
> + {
> + {0xD801,0},
> + 2,
> + "+2AE-",
> + 6
> + },
> + /* tests some more invalid UTF-16 */
> + /* (codepoint does not exist) */
> + {
> + {0xFF00,0},
> + 2,
> + "+/wA-",
> + 6
> + }
> + };
> +
> + int i;
> +
> + for (i = 0; i < sizeof(simple_tests) / sizeof(simple_tests[0]); i++)
> + {
> + char c_buffer[1024];
> + WCHAR w_buffer[1024];
Same as above, way too big.
> + int len;
> +
> + c_buffer[sizeof(c_buffer) - 1] = 0;
> + w_buffer[sizeof(w_buffer) / sizeof(WCHAR) - 1] = 0;
It probably makes more sense to initialize the buffers before each test, instead of \
one time at the beginning.
> +
> + /* test string conversion with srclen=-1 */
> + memset(c_buffer, '#', sizeof(c_buffer) - 1);
> + SetLastError(0xdeadbeef);
> + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, c_buffer, \
> sizeof(c_buffer), NULL, NULL); + if (GetLastError() == \
> ERROR_CALL_NOT_IMPLEMENTED) + {
> + skip("UTF-7 encoding not implemented\n");
> + return;
> + }
At my opinion it would be better to do one basic test at the beginning, and then \
abort immediately, instead of doing that inside of the for loop, if its not too \
complicated.
> + ok(len == simple_tests[i].utf7_len &&
> + strcmp(c_buffer, simple_tests[i].utf7) == 0 &&
> + c_buffer[len] == '#',
> + "simple_test failure i=%i dst=\"%s\" len=%i\n", i, c_buffer, len);
> + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError());
> +
> + /* test string conversion with srclen=-2 */
> + memset(c_buffer, '#', sizeof(c_buffer) - 1);
> + SetLastError(0xdeadbeef);
> + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -2, c_buffer, \
> sizeof(c_buffer), NULL, NULL); + ok(len == simple_tests[i].utf7_len &&
> + strcmp(c_buffer, simple_tests[i].utf7) == 0 &&
> + c_buffer[len] == '#',
> + "simple_test failure i=%i dst=\"%s\" len=%i\n", i, c_buffer, len);
> + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError());
You are testing a lot of things in a single ok(...) block. It will be very difficult \
to find out whats wrong if we get test failures in the future. I think it would be \
better to split that into multiple checks, probably with some more meaningful message \
about what is wrong (wrong len, wrong content, ...)
> +
> + /* test string conversion with dstlen=len-1 */
> + memset(c_buffer, '#', sizeof(c_buffer) - 1);
> + SetLastError(0xdeadbeef);
> + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, c_buffer, \
> simple_tests[i].utf7_len - 1, NULL, NULL); + ok(len == 0 &&
> + memcmp(c_buffer, simple_tests[i].utf7, simple_tests[i].utf7_len - 1) == \
> 0 && + c_buffer[simple_tests[i].utf7_len - 1] == '#',
> + "simple_test failure i=%i dst=\"%s\" len=%i\n", i, c_buffer, len);
> + ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "error=%x\n", \
> GetLastError()); +
> + /* test string conversion with dstlen=len */
> + memset(c_buffer, '#', sizeof(c_buffer) - 1);
> + SetLastError(0xdeadbeef);
> + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, c_buffer, \
> simple_tests[i].utf7_len, NULL, NULL); + ok(len == simple_tests[i].utf7_len \
> && + strcmp(c_buffer, simple_tests[i].utf7) == 0 &&
> + c_buffer[len] == '#',
> + "simple_test failure i=%i dst=\"%s\" len=%i\n", i, c_buffer, len);
> + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError());
> +
> + /* test string conversion with dstlen=len+1 */
> + memset(c_buffer, '#', sizeof(c_buffer) - 1);
> + SetLastError(0xdeadbeef);
> + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, c_buffer, \
> simple_tests[i].utf7_len + 1, NULL, NULL); + ok(len == \
> simple_tests[i].utf7_len && + strcmp(c_buffer, simple_tests[i].utf7) == 0 \
> && + c_buffer[len] == '#',
> + "simple_test failure i=%i dst=\"%s\" len=%i\n", i, c_buffer, len);
> + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError());
> +
> + /* test dry run with dst=NULL and dstlen=0 */
> + memset(c_buffer, '#', sizeof(c_buffer));
> + SetLastError(0xdeadbeef);
> + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, NULL, 0, \
> NULL, NULL); + ok(len == simple_tests[i].utf7_len &&
> + c_buffer[0] == '#',
> + "simple_test failure i=%i len=%i\n", i, len);
> + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError());
> +
> + /* test dry run with dst!=NULL and dstlen=0 */
> + memset(c_buffer, '#', sizeof(c_buffer) - 1);
> + SetLastError(0xdeadbeef);
> + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, c_buffer, \
> 0, NULL, NULL); + ok(len == simple_tests[i].utf7_len &&
> + c_buffer[0] == '#',
> + "simple_test failure i=%i len=%i\n", i, len);
> + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError());
> +
> + /* all simple utf16-to-utf7 tests can be reversed to make utf7-to-utf16 \
> tests */ + memset(w_buffer, '#', sizeof(w_buffer) - sizeof(WCHAR));
> + SetLastError(0xdeadbeef);
> + len = MultiByteToWideChar(CP_UTF7, 0, simple_tests[i].utf7, -1, w_buffer, \
> sizeof(w_buffer) / sizeof(WCHAR)); + todo_wine ok(len == \
> simple_tests[i].utf16_len && + memcmp(w_buffer, simple_tests[i].utf16, \
> len * sizeof(WCHAR)) == 0 && + w_buffer[len] == 0x2323,
> + "simple_test failure i=%i dst=%s len=%i\n", i, wine_dbgstr_w(w_buffer), \
> len); + todo_wine ok(GetLastError() == 0xdeadbeef, "error=%x\n", \
> GetLastError());
Why do you use todo_wine when the whole test block is skipped? Moreover, it should \
probably go into the later patches which add the MultibyteToWideChar tests.
> + }
> +}
> +
> static void test_undefined_byte_char(void)
> {
> static const struct tag_testset {
> @@ -618,6 +750,8 @@ START_TEST(codepage)
> test_string_conversion(NULL);
> test_string_conversion(&bUsedDefaultChar);
>
> + test_utf7_encoding();
> +
> test_undefined_byte_char();
> test_threadcp();
> }
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic