-----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