Re: [PATCH] findstr: Added test for findstr. (try 2)

[Kaipeng Zeng]

Excuse me,can anyone have a look at this patch?
I appreciate for any comment.
Thanks！


2015-03-24 13:55 GMT+08:00 Kaipeng Zeng <kaipeng94(a)gmail.com>:
> Thanks Stefan!
>
> Superseded patch 110143 .
> ChangeLog:
> - Exclude change to configure
> - Return exit code of findstr in runcmd()

[PATCH] findstr: Added test for findstr. (try 2)

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Am 2015-04-01 um 16:37 schrieb Kaipeng Zeng:
> Excuse me,can anyone have a look at this patch? I appreciate for 
> any comment. Thanks！

I think the main issue is that the test alone doesn't do much. It
should either be part of a series that gets the actual implementation
started, or a series of tests that test more than the very minimum
functionality.

Wrt the code itself, I found a few more issues:
> +    wcmd = HeapAlloc(GetProcessHeap(), 0, strlen(cmd) + 1);
> +    strcpy(wcmd, cmd);
> Please check the return value.
> +    DWORD dwBytesToWrite = (DWORD)strlen(text_for_test);
> Is the cast necessary? Do you get a warning without it?
> +    while (ReadFile(hRead, output, GetFileSize(hRead, NULL), &bytesRead, NULL))
> +        ;
> This can overflow your output buffer.
> +    file = CreateFileA("test.txt", GENERIC_READ|GENERIC_WRITE, 0, NULL,
> +                       CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
> Here it's a good idea to check for errors as well. Same for WriteFile.
> +void test_without_arg(void)
> +{
> +    char buffer[2048] = {0};
> +
> +    init_env();
> +    todo_wine ok(!runcmd("findstr free test.txt", buffer), "'findstr free test.txt' fails\n");
> +    todo_wine ok(!strcmp(buffer, "Wine will always be free software.\n"),
> +                 "Return wrong string.\n");
> +    DeleteFileA("test.txt");
> +}
> +
> +START_TEST(findstr)
> +{
> +    test_without_arg();
> +}
> init_env seems like something that should be called from the main
> function, not from the first test function. We want the individual
> tests to be independent of each other.

The output written by the ok() lines could be improved. At least print
the string and the return value findstr.exe reported. You do not have
to print the expected result if the expected result is always the same
- - ok() will print the line number.

You could also consider skipping the string comparison if findstr.exe
returns an unexpected error. It's quite unlikely the output is correct
in this situation.

Stefan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJVHU/bAAoJEN0/YqbEcdMwz+MP/izSwfPKUmGpXmA4k7q4ZTm/
XlNDuuPk4kfMaGNsO8gguYZgO4p24GOkL9weEPlB7+ny9ATXUpZIUSyNs7Gw292j
YFWlfRUsyYHst8XfCkdDD7cy9EGQBCx3LLmZV1uhGKB2xWHPvQOKY/0fJPahBzEY
TlZBkJWHbk7042wP3wmfcgbSupEw3QjmOqs1qymANqfIoDrJnD5qNsO2+DIbUdQq
EiBclOWjE3YIw5yqkrN3YEoorNi/MKdLyWRalcyW/c95CoW2IdjQoHkMSyWmaU61
mPHiDli2j+EB0UtU/EaCB2LhevB1trh9DtrjGziQ6NfFq29g7Hzh9vgid8PhWGhP
NS4bvvU5c3jGGsEAu4DmGSEqnueRtZj7lsaIMpdF8Hltigj0Z5j4AJIsq2hqlEW7
wIqLfJNPpuiqbaxv1OCAxbjWROYzufTowFUd3V8Yq8yzjfabsu1mXT6OQA4FRpTU
HNo1sZBEIujbK+Es8tSAYOxjUbk4MmswXEuxoYq66libc5AAKLZEpUMa3uwZo/9n
PC1Ro5G+UPJoz4xnBaZtyMi2kNnEySelvyb9/oba7X9v2rY3yAeAOSgUufUXN4Rq
MlttxVM79ExbyhtLFKEIcbC5T9/eN7ZCVa3TiyVXxQ2d2Eeld4YHcTlBW7MWUYxD
mPmsiQyDcRY8ZDqi6QwC
=kPaj
-----END PGP SIGNATURE-----