On Sat, 18 Sept 2021 at 17:48, Florian Eder wrote:
[snip]
diff --git a/programs/robocopy/tests/robocopy.c b/programs/robocopy/tests/robocopy.c new file mode 100644 index 00000000000..2093a990f03 --- /dev/null +++ b/programs/robocopy/tests/robocopy.c @@ -0,0 +1,63 @@ +/* + * Copyright 2021 Florian Eder + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <wine/test.h> +#define WIN32_LEAN_AND_MEAN +#include <windows.h> +#include <wchar.h>
Traditionally, we put Windows headers first, then Wine headers after. Is there a reason you decided to use WCHARs? You might find using plain char easier to work as you build your unit tests.
+static DWORD execute_robocopy(const WCHAR *cmd_line) +{ + STARTUPINFOW startup_info; + PROCESS_INFORMATION process_info; + DWORD exit_code; + WCHAR cmd_line_copy[2048];
Why so much memory? And you can shorten the variable name to cmd or similar.
+ memset(&startup_info, 0, sizeof(STARTUPINFOW)); + startup_info.dwFlags = STARTF_USESTDHANDLES;
Where are you setting hStdInput, hStdOutput, and hStdError?
+ /* CreateProcess must not be called with static strings */
We don't need this comment.
+ wcscpy(cmd_line_copy, cmd_line); + + if (!CreateProcessW(NULL, cmd_line_copy, NULL, NULL, TRUE, 0, NULL, NULL, &startup_info, &process_info)) + return -1; + + if (WaitForSingleObject(process_info.hProcess, 30000) == WAIT_TIMEOUT) + return -1;
Why are you returning a negative integer from a function returning type DWORD? While this will work, it's better not to do this.
+ GetExitCodeProcess(process_info.hProcess, &exit_code); + + CloseHandle(process_info.hThread); + CloseHandle(process_info.hProcess); + + return exit_code; +}
You'll probably find it a lot harder to constantly check the exit code from the caller with if statements. You might change this later, I haven't looked yet.
+START_TEST(robocopy) +{ + WCHAR temp_path[4096]; + + ok(GetTempPathW(ARRAY_SIZE(temp_path), temp_path) != 0, "couldn't get temp folder path");
I don't see why GetTempPath() and SetCurrentDirectory() are in this patch, as they're not used.
+ /* robocopy is only available from Vista onwards, abort test if not available */ + if (execute_robocopy(L"robocopy.exe") == -1) return;
This sort of test should be first. Stylistically, naming a function 'execute_robocopy', then adding 'robocopy' as a command is going to be cumbersome. (And I know, the reg.exe tests are similar... if shorter in name).
+ ok(SetCurrentDirectoryW(temp_path), "couldn't set CWD to temp folder \"%S\"", temp_path); + /* TODO: conformance tests here */
Okay. Perhaps you could add some basic syntax tests instead of a comment?
+} -- 2.32.0