[PATCH 1/2] shell32: SHFileOperation: test added [try 2]

[James Hawkins]

2008/6/18 Vitaly Perov <vitperov at etersoft.ru>:
> Changelog:
>  - shell32: test added. It check whether SHFileOperation could correctly work
> with input files mask '*.*'
>

Couple problems:

--- a/dlls/shell32/tests/shlfileop.c
+++ b/dlls/shell32/tests/shlfileop.c
@@ -120,6 +120,7 @@ static void init_shfo_tests(void)
     CreateDirectoryA("testdir2", NULL);
     CreateDirectoryA("testdir2\\nested", NULL);
     createTestFile("testdir2\\one.txt");
+    createTestFile("testdir2\\one2.txt");
     createTestFile("testdir2\\nested\\two.txt");
 }

@@ -136,11 +137,14 @@ static void clean_after_shfo_tests(void)
     DeleteFileA("test4.txt\\test3.txt");
     RemoveDirectoryA("test4.txt");
     DeleteFileA("testdir2\\one.txt");
+    DeleteFileA("testdir2\\one2.txt");
     DeleteFileA("testdir2\\test1.txt");
     DeleteFileA("testdir2\\test2.txt");
     DeleteFileA("testdir2\\test3.txt");
     DeleteFileA("testdir2\\test4.txt\\test1.txt");
     DeleteFileA("testdir2\\nested\\two.txt");
+    DeleteFileA("testdir2\\nested\\one.txt");
+    DeleteFileA("testdir2\\nested\\one2.txt");
     RemoveDirectoryA("testdir2\\test4.txt");
     RemoveDirectoryA("testdir2\\nested");
     RemoveDirectoryA("testdir2");

Why are you adding another test file?  There are already multiple *.*
files created in the test directory.

+    /* try moving with MASK '*.*'  */
+    shfo.pFrom = "testdir2\\*.*\0";
+    shfo.pTo = "testdir2\\nested\0";
+    retval = SHFileOperationA(&shfo);
+    ok(retval == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", retval);
+    ok(!file_exists("testdir2\\one.txt"), "Expected one.txt to not exist\n");
+    ok(!file_exists("testdir2\\one2.txt"), "Expected one2.txt to not exist\n");
+    ok(file_exists("testdir2\\nested\\one.txt"), "Expected one.txt to
exist\n");
+    ok(file_exists("testdir2\\nested\\one2.txt"), "Expected one.txt
to exist\n");

This test is misleading.  We already test for moving multiple files to
a destination directory using a mask: line 1102 of the git version of
this file.  The misleading part is that what we don't test is moving
multiple files to a destination directory when the FOF_MULTIDESTFILES
flag is set (it's set in a previous test and never unset in your
patch).  This would be a much better addition to the tests (and easier
to understand because it follows from the previous test):

/* move files using glob to a dest dir with FOF_MULTIDESTFILES */
shfo.fFlags |= FOF_MULTIDESTFILES;
set_curr_dir_path(from, "test?.txt\0");
set_curr_dir_path(to, "testdir2\0");
ok(!file_exists("testdir2\\test2.txt"), "The file is not moved yet\n");
ok(!file_exists("testdir2\\test4.txt"), "The directory is not moved yet\n");
retval = SHFileOperationA(&shfo);
ok(retval == ERROR_SUCCESS, "Files and directories are moved to directory\n");
ok(file_exists("testdir2\\test2.txt"), "The file is moved\n");
ok(file_exists("testdir2\\test4.txt"), "The directory is moved\n");
ok(file_exists("testdir2\\test4.txt\\test1.txt"), "The file in
subdirectory is moved\n");


This should immediately follow the similar test on line 1102.

-- 
James Hawkins