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