New subject: [PATCH v6 0/2] MR7060: scrrun/tests: Add drive_get_path test

29 Dec 2024

Thanks for the patches. The implementation looks good and it allows the Windows Search Index Analyzer to display its initial dialog. It's possible that drive_get_Path should really call GetVolumePathNameW and strip the trailing backslash, but Wine's implementation of the Drive class currently only supports letter drives, so a simpler implementation is fine for now.

I'm not sure that all the tests are necessary. If it were me, I would just do this:

```diff

diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c
index e53bd092ce9..aabec74b3f9 100644
--- a/dlls/scrrun/tests/filesystem.c
+++ b/dlls/scrrun/tests/filesystem.c
@@ -1471,7 +1471,7 @@ static void test_DriveCollection(void)
     while (IEnumVARIANT_Next(enumvar, 1, &var, &fetched) == S_OK) {
         IDrive *drive = (IDrive*)V_DISPATCH(&var);
         DriveTypeConst type;
-        BSTR str;
+        BSTR str, path;
 
         hr = IDrive_get_DriveType(drive, &type);
         ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
@@ -1482,7 +1482,17 @@ static void test_DriveCollection(void)
         hr = IDrive_get_DriveLetter(drive, &str);
         ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         ok(SysStringLen(str) == 1, "got string %s\n", wine_dbgstr_w(str));
+
+        hr = IDrive_get_Path(drive, NULL);
+        ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr);
+
+        hr = IDrive_get_Path(drive, &path);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        ok(SysStringLen(path) == 2 && path[0] == str[0] && path[1] == ':',
+           "got string %s\n", wine_dbgstr_w(path));
+
         SysFreeString(str);
+        SysFreeString(path);
 
         hr = IDrive_get_IsReady(drive, NULL);
         ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr);
```

However, when submitting tests I am often told that I have added either too few tests or too many tests, and I'm not sure what the preferred amount of tests is here.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7060#note_91192

    

Re: [PATCH v6 0/2] MR7060: scrrun/tests: Add drive_get_path test

Alex Henrie (＠alexhenrie)

Marvin

tags

participants (2)