On 9/6/21 9:54 AM, Florian Eder wrote:
Basic files and required changes to configure(.ac) to create a scaffolding for the conformance tests for the robocopy utility
Signed-off-by: Florian Eder others.meder@gmail.com
configure | 1 + configure.ac | 1 + programs/robocopy/tests/Makefile.in | 5 ++ programs/robocopy/tests/robocopy.c | 80 +++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+) create mode 100644 programs/robocopy/tests/Makefile.in create mode 100644 programs/robocopy/tests/robocopy.c
diff --git a/configure b/configure index 9e2fa9ef009..297b1783575 100755 --- a/configure +++ b/configure @@ -21265,6 +21265,7 @@ wine_fn_config_makefile programs/regini enable_regini wine_fn_config_makefile programs/regsvcs enable_regsvcs wine_fn_config_makefile programs/regsvr32 enable_regsvr32 wine_fn_config_makefile programs/robocopy enable_robocopy +wine_fn_config_makefile programs/robocopy/tests enable_tests wine_fn_config_makefile programs/rpcss enable_rpcss wine_fn_config_makefile programs/rundll.exe16 enable_win16 wine_fn_config_makefile programs/rundll32 enable_rundll32 diff --git a/configure.ac b/configure.ac index 88b9426260f..c8aa29b6628 100644 --- a/configure.ac +++ b/configure.ac @@ -3964,6 +3964,7 @@ WINE_CONFIG_MAKEFILE(programs/regini) WINE_CONFIG_MAKEFILE(programs/regsvcs) WINE_CONFIG_MAKEFILE(programs/regsvr32) WINE_CONFIG_MAKEFILE(programs/robocopy) +WINE_CONFIG_MAKEFILE(programs/robocopy/tests) WINE_CONFIG_MAKEFILE(programs/rpcss) WINE_CONFIG_MAKEFILE(programs/rundll.exe16,enable_win16) WINE_CONFIG_MAKEFILE(programs/rundll32) diff --git a/programs/robocopy/tests/Makefile.in b/programs/robocopy/tests/Makefile.in new file mode 100644 index 00000000000..146246e4b1a --- /dev/null +++ b/programs/robocopy/tests/Makefile.in @@ -0,0 +1,5 @@ +TESTDLL = robocopy.exe +IMPORTS = user32
Why does this import user32?
+C_SRCS = \
- robocopy.c
diff --git a/programs/robocopy/tests/robocopy.c b/programs/robocopy/tests/robocopy.c new file mode 100644 index 00000000000..ca3d8d7da42 --- /dev/null +++ b/programs/robocopy/tests/robocopy.c @@ -0,0 +1,80 @@ +/*
- 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>
+static DWORD execute_robocopy(const WCHAR *command_line, DWORD expected_exit_code) +{
- STARTUPINFOW startup_information;
- PROCESS_INFORMATION process_information;
- DWORD return_value;
- DWORD process_exit_code;
- WCHAR command_line_copy[2048];
This is a bit of a nitpick, but these variable names are more verbose than they need to be. E.g. "return_value" is usually just spelled "ret" elsewhere. "startup_information" can be "startup_info".
- memset(&startup_information, 0, sizeof(STARTUPINFOW));
- startup_information.dwFlags = STARTF_USESTDHANDLES;
- /* CreateProcess must not be called with static strings */
- wcscpy(command_line_copy, command_line);
- if (!CreateProcessW(NULL, command_line_copy, NULL, NULL, TRUE, CREATE_NO_WINDOW, NULL, NULL, &startup_information, &process_information))
What's the purpose of CREATE_NO_WINDOW here?
This line is also very long. We don't have a set line length, but I'd recommend breaking after 120 characters, if not sooner.
- {
ok(expected_exit_code == -1, "process execution of \"%S\" failed with error %d\n", command_line_copy, GetLastError());return -1;- }
- return_value = WaitForSingleObject(process_information.hProcess, 30000);
- if (return_value == WAIT_TIMEOUT)
- {
TerminateProcess(process_information.hProcess, 1);ok(FALSE, "process (executing \"%S\") timed out\n", command_line_copy);
ok(FALSE) is something of an antipattern; you generally want ok(!return_value) instead.
Personally I also don't think it's worth trying to clean up like this if you're going to print a failure message anyway, especially if there's no reasonable way this should fail. A test failure is a test failure; it needs to be fixed either way.
return -1;- }
- GetExitCodeProcess(process_information.hProcess, &process_exit_code);
- CloseHandle(process_information.hThread);
- CloseHandle(process_information.hProcess);
- /* also accept any exit code if expected exit code is -1 */
- ok((expected_exit_code == process_exit_code) || (expected_exit_code == -1),
"process exit code was %d, but expected %d\n", process_exit_code, expected_exit_code);
Usually I'd prefer to just move the ok() to the caller.
- return process_exit_code;
+}
+START_TEST(robocopy) +{
- WCHAR previous_cwd_path[4096], temp_path[4096];
- ok(GetFullPathNameW(L".", ARRAY_SIZE(previous_cwd_path), previous_cwd_path, NULL) != 0, "couldn't get CWD path");
- ok(GetTempPathW(ARRAY_SIZE(temp_path), temp_path) != 0, "couldn't get temp folder path");
- /* robocopy is only available from Vista onwards, abort test if not available */
- if (execute_robocopy(L"robocopy.exe", -1) == -1) return;
- /* set CWD to temp folder */
This comment is redundant; I can see that from the next line.
- ok(SetCurrentDirectoryW(temp_path), "couldn't set CWD to temp folder "%S"", temp_path);
- /* TODO: conformance tests here */
- /* Reset CWD to previous folder */
- ok(SetCurrentDirectoryW(previous_cwd_path), "couldn't set CWD to previous CWD folder "%S"", previous_cwd_path);
+} \ No newline at end of file
Whitespace error.