[PATCH v6 6/6] robocopy/tests: Add basic conformance tests
Zebediah Figura (she/her)
zfigura at codeweavers.com
Fri Oct 8 01:04:06 CDT 2021
On 9/21/21 09:54, Florian Eder wrote:
> Basic conformance tests create a test source directory with
> files and execute robocopy on it, checking whether the resulting
> destination directory, the remaining source directory and the exit code
> is as expected
>
> Signed-off-by: Florian Eder <others.meder at gmail.com>
> ---
> v6: exit_code is now set as a pointer / argument to execute_robocopy, instead
> of being its return value
> ---
> programs/robocopy/tests/robocopy.c | 188 ++++++++++++++++++++++++++++-
> 1 file changed, 187 insertions(+), 1 deletion(-)
>
> diff --git a/programs/robocopy/tests/robocopy.c b/programs/robocopy/tests/robocopy.c
> index d5d3dee473a..0eb975861bc 100644
> --- a/programs/robocopy/tests/robocopy.c
> +++ b/programs/robocopy/tests/robocopy.c
> @@ -49,10 +49,153 @@ static BOOL execute_robocopy(const WCHAR *args, DWORD *exit_code)
> return TRUE;
> }
>
> +static void create_test_file(const WCHAR *relative_path, size_t size, LONGLONG fixed_filetime, long filetime_offset)
> +{
> + HANDLE handle;
> + WCHAR path[1024];
> + wcscpy(path, L"\\\\?\\");
> + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4]));
> + wcscat(path, relative_path);
> + handle = CreateFileW(path, FILE_GENERIC_WRITE | FILE_GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_ALWAYS, 0, NULL);
> + ok(handle != INVALID_HANDLE_VALUE, "creation of %s failed (0x%08x)\n", debugstr_w(path), GetLastError());
> + if (size != 0)
> + {
> + BYTE *data;
> + DWORD bytes_written;
> + data = calloc(size, sizeof(BYTE));
> + ok(WriteFile(handle, data, size, &bytes_written, NULL), "writing to %s failed (%d)\n", debugstr_w(path), GetLastError());
> + }
> + if (fixed_filetime != 0)
> + {
> + FILETIME time;
> + LARGE_INTEGER time_as_integer;
> + time_as_integer.QuadPart = fixed_filetime;
> + time.dwHighDateTime = time_as_integer.HighPart;
> + time.dwLowDateTime = time_as_integer.LowPart;
> + ok(SetFileTime(handle, &time, &time, &time), "filetime manipulation of %s failed (%d)\n", debugstr_w(path), GetLastError());
> + }
> + if (filetime_offset != 0)
> + {
> + FILETIME time, modified_time, access_time;
> + LARGE_INTEGER time_as_integer;
> + GetFileTime(handle, &time, &modified_time, &access_time);
> + /* FILETIME is no union with LONGLONG / LONG64, casting could be unsafe */
> + time_as_integer.HighPart = time.dwHighDateTime;
> + time_as_integer.LowPart = time.dwLowDateTime;
> + /* 1000 * 1000 * 60 * 60 * 24 = 86400000000ns per day */
> + time_as_integer.QuadPart += 864000000000LL * filetime_offset;
> + time.dwHighDateTime = time_as_integer.HighPart;
> + time.dwLowDateTime = time_as_integer.LowPart;
> + ok(SetFileTime(handle, &time, &time, &time), "filetime manipulation of %s failed (%d)\n", debugstr_w(path), GetLastError());
> + }
You don't actually test the file times yet; this probably deserves to be
put off to a later patch.
> + CloseHandle(handle);
> +}
> +
> +static void create_test_folder(const WCHAR *relative_path)
> +{
> + WCHAR path[1024];
> + wcscpy(path, L"\\\\?\\");
> + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4]));
For tests I think it's reasonable to assume everything fits in MAX_PATH.
> + wcscat(path, relative_path);
> + CreateDirectoryW(path, NULL);
> +}
> +
> +static void create_test_source_folder(void)
> +{
> + create_test_folder(L"robocopy_source");
> + create_test_folder(L"robocopy_source\\folderA");
> + create_test_folder(L"robocopy_source\\folderB");
> + create_test_folder(L"robocopy_source\\folderC");
> + create_test_folder(L"robocopy_source\\folderA\\folderD");
> + create_test_folder(L"robocopy_source\\folderA\\folderE");
> + create_test_file(L"robocopy_source\\fileA.a", 4000, 0, -10);
> + create_test_file(L"robocopy_source\\fileB.b", 8000, 0, -2);
> + create_test_file(L"robocopy_source\\folderA\\fileC.c", 60, 0, -2);
> + create_test_file(L"robocopy_source\\folderA\\fileD.d", 80, 0, 0);
> + create_test_file(L"robocopy_source\\folderA\\folderD\\fileE.e", 10000, 0, -10);
> + create_test_file(L"robocopy_source\\folderB\\fileF.f", 10000, 132223104000000000, 0);
> + create_test_file(L"robocopy_source\\folderB\\fileG.g", 200, 132223104000000000, 0);
> +}
> +
> +static void check_file_and_delete(const WCHAR *relative_path, BOOL should_exist)
> +{
> + WCHAR path[1024];
> + wcscpy(path, L"\\\\?\\");
> + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4]));
> + wcscat(path, relative_path);
> + if (!DeleteFileW(path))
> + {
> + if (GetLastError() == ERROR_FILE_NOT_FOUND)
> + ok(!should_exist, "file \"%s\" does not exist, but should exist\n", debugstr_w(relative_path));
> + else if (GetLastError() == ERROR_PATH_NOT_FOUND)
> + ok(!should_exist, "file \"%s\" and the parent directory do not exist, but should exist\n", debugstr_w(relative_path));
> + else
> + ok(FALSE, "file \"%s\" DeleteFileW returned error %d\n", debugstr_w(relative_path), GetLastError());
> + }
> + else
> + {
> + ok(should_exist, "file \"%s\" should not exist, but does exist\n", debugstr_w(relative_path));
> + }
This seems structured very awkwardly, and the fact that you have
ok(FALSE) is good evidence of that. I'd suggest something like the
following:
ret = DeleteFileW(path);
ok(ret == should_exist, ...);
if (!should_exist)
ok(GetLastError() == ERROR_FILE_NOT_FOUND || GetLastError() ==
ERROR_PATH_NOT_FOUND, ...);
Similarly for check_folder_and_delete().
I might also get rid of the and_delete from the name.
> +}
> +
> +static void check_folder_and_delete(const WCHAR *relative_path, BOOL should_exist)
> +{
> + WCHAR path[1024];
> + wcscpy(path, L"\\\\?\\");
> + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4]));
> + wcscat(path, relative_path);
> + if (!RemoveDirectoryW(path))
> + {
> + if (GetLastError() == ERROR_FILE_NOT_FOUND)
> + ok(!should_exist, "directory \"%s\" does not exist, but should exist\n", debugstr_w(relative_path));
> + else if (GetLastError() == ERROR_PATH_NOT_FOUND)
> + ok(!should_exist, "directory \"%s\" and the parent directory do not exist, but should exist\n", debugstr_w(relative_path));
> + else if (GetLastError() == ERROR_DIR_NOT_EMPTY)
> + ok(FALSE, "directory \"%s\" is unexpectedly not empty\n", debugstr_w(relative_path));
> + else
> + ok(FALSE, "directory \"%s\" DeleteFileW returned error %d\n", debugstr_w(relative_path), GetLastError());
> + }
> + else
> + {
> + ok(should_exist, "directory \"%s\" should not exist, but does exist\n", debugstr_w(relative_path));
> + }
> +}
> +
> +static void check_basic_copy_test(void)
> +{
> + check_file_and_delete(L"robocopy_source\\fileA.a", TRUE);
> + check_file_and_delete(L"robocopy_source\\fileB.b", TRUE);
> + check_file_and_delete(L"robocopy_source\\folderA\\fileC.c", TRUE);
> + check_file_and_delete(L"robocopy_source\\folderA\\fileD.d", TRUE);
> + check_file_and_delete(L"robocopy_source\\folderA\\folderD\\fileE.e", TRUE);
> + check_file_and_delete(L"robocopy_source\\folderB\\fileF.f", TRUE);
> + check_file_and_delete(L"robocopy_source\\folderB\\fileG.g", TRUE);
> + check_folder_and_delete(L"robocopy_source\\folderA\\folderD", TRUE);
> + check_folder_and_delete(L"robocopy_source\\folderA\\folderE", TRUE);
> + check_folder_and_delete(L"robocopy_source\\folderA", TRUE);
> + check_folder_and_delete(L"robocopy_source\\folderB", TRUE);
> + check_folder_and_delete(L"robocopy_source\\folderC", TRUE);
> + check_folder_and_delete(L"robocopy_source", TRUE);
> +
> + check_file_and_delete(L"robocopy_destination\\fileA.a", TRUE);
> + check_file_and_delete(L"robocopy_destination\\fileB.b", TRUE);
> + todo_wine check_file_and_delete(L"robocopy_destination\\folderA\\fileC.c", FALSE);
> + todo_wine check_file_and_delete(L"robocopy_destination\\folderA\\fileD.d", FALSE);
> + todo_wine check_file_and_delete(L"robocopy_destination\\folderA\\folderD\\fileE.e", FALSE);
> + todo_wine check_file_and_delete(L"robocopy_destination\\folderB\\fileF.f", FALSE);
> + todo_wine check_file_and_delete(L"robocopy_destination\\folderB\\fileG.g", FALSE);
> + todo_wine check_folder_and_delete(L"robocopy_destination\\folderA\\folderD", FALSE);
> + check_folder_and_delete(L"robocopy_destination\\folderA\\folderE", FALSE);
> + todo_wine check_folder_and_delete(L"robocopy_destination\\folderA", FALSE);
> + todo_wine check_folder_and_delete(L"robocopy_destination\\folderB", FALSE);
> + check_folder_and_delete(L"robocopy_destination\\folderC", FALSE);
> + check_folder_and_delete(L"robocopy_destination", TRUE);
> +}
> +
> START_TEST(robocopy)
> {
> DWORD exit_code;
> - WCHAR temp_path[MAX_PATH];
> + WCHAR temp_cmd[512], temp_path[MAX_PATH];
>
> /* robocopy is only available from Vista onwards, abort test if not available */
> if (!execute_robocopy(L"", &exit_code)) return;
> @@ -100,4 +243,47 @@ START_TEST(robocopy)
> execute_robocopy(L"invalid_folder /?", &exit_code);
> ok(exit_code == 16, "unexpected exit code %d\n", exit_code);
> winetest_pop_context();
> +
> + winetest_push_context("basic copy test 1");
> + create_test_source_folder();
> + execute_robocopy(L"robocopy_source robocopy_destination /r:1 /w:0", &exit_code);
You might consider shortening things by changing the working directory
to a generated subdir of %temp% instead of %temp% itself, and then you
don't have to worry about prepending "robocopy" to everything.
> + ok(exit_code == 1, "unexpected exit code %d\n", exit_code);
> + check_basic_copy_test();
> + winetest_pop_context();
> +
> + winetest_push_context("basic copy test 2");
> + create_test_source_folder();
> + execute_robocopy(L"./robocopy_source third_folder/../robocopy_destination /r:1 /w:0", &exit_code);
> + ok(exit_code == 1, "unexpected exit code %d\n", exit_code);
> + check_basic_copy_test();
> + winetest_pop_context();
> +
> + winetest_push_context("basic copy test 3");
> + create_test_source_folder();
> + create_test_folder(L"robocopy_destination");
> + create_test_file(L"robocopy_destination\\fileA.a", 9000, 0, -10);
> + execute_robocopy(L"./robocopy_source robocopy_source/../robocopy_destination /r:1 /w:0", &exit_code);
> + ok(exit_code == 1, "unexpected exit code %d\n", exit_code);
> + check_basic_copy_test();
> + winetest_pop_context();
> +
> + winetest_push_context("basic copy test 4");
> + create_test_source_folder();
> + swprintf(temp_cmd, ARRAY_SIZE(temp_cmd),
> + L"%s\\robocopy_source %s\\robocopy_destination /r:1 /w:0",
> + temp_path, temp_path);
> + execute_robocopy(temp_cmd, &exit_code);
> + ok(exit_code == 1, "unexpected exit code %d\n", exit_code);
> + check_basic_copy_test();
> + winetest_pop_context();
> +
> + winetest_push_context("basic copy test 5");
> + create_test_source_folder();
> + swprintf(temp_cmd, ARRAY_SIZE(temp_cmd),
> + L"%s\\third_folder\\..\\robocopy_source %s\\third_folder\\..\\robocopy_destination /r:1 /w:0",
> + temp_path, temp_path);
> + execute_robocopy(temp_cmd, &exit_code);
> + ok(exit_code == 1, "unexpected exit code %d\n", exit_code);
> + check_basic_copy_test();
> + winetest_pop_context();
> }
>
There's a lot of behaviour covered in patch 5 that isn't covered here.
For example:
* There are no tests for path parameters (i.e. after the source and
dest). When adding such tests, I would recommend making sure you test
the following:
- specifying the same path more than once;
- specifying an absolute path;
- specifying a path that doesn't exist in the source;
- specifying both valid and invalid paths, in both orders, to see
whether partial copies are done;
- specifying wildcard paths [also, wildcard paths followed by a
backslash, like a/*/b? I'm assuming this is why we need to use
PathMatchSpec()...]
* What happens if the source folder itself doesn't exist?
* Testing what happens if the source and destination are the same seems
helpful.
I believe you have most of these tests, somewhere, but ideally they
should all be before patch 5/6 in this series :-)
More information about the wine-devel
mailing list