[PATCH] shell32/tests Add SHCreateDirectoryExW with Hwnd ALREADY_EXISTS

[John Thomson]

Test for conformance of SHCreateDirectoryExW with a Hwnd
when target directory already exists

Signed-off-by: John Thomson 
---
As part of testing
[PATCH] shell32: SHCreateDirectoryExW no CANCELLED for ALREADY_EXISTS
---
 dlls/shell32/tests/shlfileop.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c
index 8fb4dcf66b..25eb05f440 100644
--- a/dlls/shell32/tests/shlfileop.c
+++ b/dlls/shell32/tests/shlfileop.c
@@ -2543,6 +2543,7 @@ static void test_unicode(void)
     int ret;
     HANDLE file;
     static const WCHAR UNICODE_PATH_TO[] = {'c',':','\',0x00ae,0x00ae,'\0'};
+    HWND hwnd;

shfoW.hwnd = NULL;
     shfoW.wFunc = FO_DELETE;
@@ -2633,6 +2634,21 @@ static void test_unicode(void)
     ok(GetLastError() == ERROR_SUCCESS ||
        broken(GetLastError() == ERROR_INVALID_HANDLE), /* WinXp, win2k3 */
        "Expected ERROR_SUCCESS, got %d\n", GetLastError());
+
+    /* Check SHCreateDirectoryExW with a Hwnd
+     * returns ERROR_ALREADY_EXISTS where a directory already exists */
+    /* Get any window handle */
+    hwnd = FindWindowA(NULL, NULL);
+    ok(hwnd, "FindWindowA failed to produce a hwnd");
+    ret = SHCreateDirectoryExW(hwnd, UNICODE_PATH, NULL);
+    ok(!ret, "SHCreateDirectoryExW returned %d\n", ret);
+    /* Create already-existing directory */
+    ok(file_existsW(UNICODE_PATH), "The directory was not created\n");
+    ret = SHCreateDirectoryExW(hwnd, UNICODE_PATH, NULL);
+    ok(ret == ERROR_ALREADY_EXISTS,
+       "SHCreateDirectoryExW returned %d. Expected %d",
+       ret, ERROR_ALREADY_EXISTS);
+    RemoveDirectoryW(UNICODE_PATH);
 }

static void

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=50984

Your paranoid android.

=== debian9 (32 bit report) ===

shell32:
shlfileop.c:2648: Test failed: SHCreateDirectoryExW returned 1223. Expected 183002c:shlfileop: 807 tests executed (24 marked as todo, 1 failure), 1 skipped.

Report errors:
shell32:shlfileop has no test summary line (early exit of the main process?)
shell32:shlfileop has unaccounted for failure messages
shell32:shlfileop has unaccounted for todo messages
shell32:shlfileop has unaccounted for skip messages

=== debian9 (32 bit French report) ===

shell32:
shlfileop.c:2648: Test failed: SHCreateDirectoryExW returned 1223. Expected 183002a:shlfileop: 807 tests executed (24 marked as todo, 1 failure), 1 skipped.

Report errors:
shell32:shlfileop has no test summary line (early exit of the main process?)
shell32:shlfileop has unaccounted for failure messages
shell32:shlfileop has unaccounted for todo messages
shell32:shlfileop has unaccounted for skip messages

=== debian9 (32 bit Japanese:Japan report) ===

shell32:
shlfileop.c:2648: Test failed: SHCreateDirectoryExW returned 1223. Expected 183002c:shlfileop: 807 tests executed (24 marked as todo, 1 failure), 1 skipped.

Report errors:
shell32:shlfileop has no test summary line (early exit of the main process?)
shell32:shlfileop has unaccounted for failure messages
shell32:shlfileop has unaccounted for todo messages
shell32:shlfileop has unaccounted for skip messages

=== debian9 (32 bit Chinese:China report) ===

shell32:
shlfileop.c:2648: Test failed: SHCreateDirectoryExW returned 1223. Expected 183002d:shlfileop: 807 tests executed (24 marked as todo, 1 failure), 1 skipped.

Report errors:
shell32:shlfileop has no test summary line (early exit of the main process?)
shell32:shlfileop has unaccounted for failure messages
shell32:shlfileop has unaccounted for todo messages
shell32:shlfileop has unaccounted for skip messages

=== debian9 (32 bit WoW report) ===

shell32:
shlfileop.c:2648: Test failed: SHCreateDirectoryExW returned 1223. Expected 183002b:shlfileop: 807 tests executed (24 marked as todo, 1 failure), 1 skipped.

Report errors:
shell32:shlfileop has no test summary line (early exit of the main process?)
shell32:shlfileop has unaccounted for failure messages
shell32:shlfileop has unaccounted for todo messages
shell32:shlfileop has unaccounted for skip messages

=== debian9 (64 bit WoW report) ===

shell32:
shlfileop.c:2648: Test failed: SHCreateDirectoryExW returned 1223. Expected 183002d:shlfileop: 807 tests executed (24 marked as todo, 1 failure), 1 skipped.

Report errors:
shell32:shlfileop has no test summary line (early exit of the main process?)
shell32:shlfileop has unaccounted for failure messages
shell32:shlfileop has unaccounted for todo messages
shell32:shlfileop has unaccounted for skip messages

[Francois Gouget]

This ok() call is wrong: it is missing the trailing '\n'.

+    ok(ret == ERROR_ALREADY_EXISTS,
+       "SHCreateDirectoryExW returned %d. Expected %d",
+       ret, ERROR_ALREADY_EXISTS);

Due to the missing trailing '\n' the test's "summary line" is 
concatenated to the end of the last error. It's the part that starts 
with "002c:shlfileop: 807 tests executed..."
> === debian9 (32 bit report) ===
>
> shell32:
> shlfileop.c:2648: Test failed: SHCreateDirectoryExW returned 1223. Expected 183002c:shlfileop: 807 tests executed (24 marked as todo, 1 failure), 1 skipped.

Because it's not at the start of the line the TestBot does not 
recognize it and this is what causes these additional failures:
> Report errors:
> shell32:shlfileop has no test summary line (early exit of the main process?)
> shell32:shlfileop has unaccounted for failure messages
> shell32:shlfileop has unaccounted for todo messages
> shell32:shlfileop has unaccounted for skip messages

So the fix is to:
* Add the trailing '\n'.
* Either fix the test or add a todo_wine so it does not fail on Wine.

* And for bonus points fix Wine's implementation if it is indeed wrong.

[John Thomson]

On Wed, 17 Apr 2019, at 07:00, Francois Gouget wrote:
> This ok() call is wrong: it is missing the trailing '\n'.
> So the fix is to:
> - Add the trailing '\n'.
> - Either fix the test or add a todo_wine so it does not fail on Wine.
> - And for bonus points fix Wine's implementation if it is indeed wrong.

Thanks.
Yes, I do believe this test showed Wine's implementation differs to Windows,
given that the Windows tests passed, and the Wine test failed.
As long as my test logic is sound?

This test patch is included as part of my updated fix:
shell32: SHCreateDirectoryExW no CANCELLED for ALREADY_EXISTS

Cheers,