[PATCH 0/1] MR5258: d3d9/tests: Skip desktop window tests if device creation fails.

[Robin Kertels (＠K0bin)]

DXVK fails to create a device for the desktop window which causes the test to crash.

Instead of crashing, it should just skip the remaining desktop window tests if device creation fails.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5258

[PATCH 1/1] d3d9/tests: Skip desktop window tests if device creation fails.

[Robin Kertels]

From: Robin Kertels <robin.kertels(a)gmail.com>

Signed-off-by: Robin Kertels <robin.kertels(a)gmail.com>
---
 dlls/d3d9/tests/visual.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c
index 2d54920ddba..a1f8c59c50f 100644
--- a/dlls/d3d9/tests/visual.c
+++ b/dlls/d3d9/tests/visual.c
@@ -26438,16 +26438,23 @@ static void test_desktop_window(void)
     device = create_device(d3d, GetDesktopWindow(), GetDesktopWindow(), TRUE);
     ok(!!device, "Failed to create a D3D device.\n");
 
-    hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffff0000, 1.0f, 0);
-    ok(SUCCEEDED(hr), "Failed to clear, hr %#lx.\n", hr);
-    color = getPixelColor(device, 1, 1);
-    ok(color == 0x00ff0000, "Got unexpected color 0x%08x.\n", color);
+    if (device)
+    {
+        hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffff0000, 1.0f, 0);
+        ok(SUCCEEDED(hr), "Failed to clear, hr %#lx.\n", hr);
+        color = getPixelColor(device, 1, 1);
+        ok(color == 0x00ff0000, "Got unexpected color 0x%08x.\n", color);
 
-    hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL);
-    ok(SUCCEEDED(hr), "Failed to present, hr %#lx.\n", hr);
+        hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL);
+        ok(SUCCEEDED(hr), "Failed to present, hr %#lx.\n", hr);
 
-    refcount = IDirect3DDevice9_Release(device);
-    ok(!refcount, "Device has %lu references left.\n", refcount);
+        refcount = IDirect3DDevice9_Release(device);
+        ok(!refcount, "Device has %lu references left.\n", refcount);
+    }
+    else
+    {
+        skip("Failed to create a D3D device for the desktop window, skipping tests.\n");
+    }
 
     /* test device with NULL HWND */
     device = create_device(d3d, NULL, NULL, TRUE);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/5258

[Zebediah Figura (＠zfigura)]

What is the end goal here?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5258#note_63862

[Robin Kertels (＠K0bin)]

I'm trying to reduce the amount of test failures of DXVK.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5258#note_63868

[Zebediah Figura (＠zfigura)]

I guess what I'm asking is, how are you running the tests? Manually? Automatically? Do you intend to have all the tests pass? Are you interested in collaborating on tests?

For instance, if your end goal is to have all the tests pass, I'm not immediately sure what this kind of commit gets you, since there's still a failure to fix. If you're running CI, it's not going to be useful if you have a bunch of failing tests. I don't see any value in this kind of thing in general—a failure is a failure.

On the other hand, if you're not just interested in using the existing tests, but also in collaborating on tests and helping to contribute to the test suite, I'm certainly open to adding e.g. todo_dxvk annotations and specifically accounting for DXVK-specific failures.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5258#note_64118

[Robin Kertels (＠K0bin)]

Right now I'm running them manually. Maybe we'll set up CI once most tests pass. I am interested in collaborating on tests.

One of the main reasons why I'd like to land this MR is that I'm not entirely sure it's possible to make rendering to the desktop work. It would be useful to have the D3D9 tests be able to run with DXVK on Windows and creating a Vulkan swapchain for the desktop window fails with VK_ERROR_INITIALIZATION_FAILED.

todo_dxvk would work too, I guess. I personally think skipping if device creation fails is cleaner.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5258#note_64128

[Zebediah Figura (＠zfigura)]

So normally I object to this kind of patch in the context of "fixing" Wine tests, since it doesn't really fix anything—a failure is a failure. This is obviously a bit of a different situation though, but I guess the point is that this is something that I'd sign off on only for dxvk's benefit, not because it'd be desirable "anyway".

I'm still curious what the end goal will look like here—this patch makes a lot more sense if we also have "todo_dxvk" annotations in tree everywhere—but if it helps for now I'm not opposed to accepting anyway, at least until we work that out.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5258#note_64773

[PATCH 0/1] MR5258: d3d9/tests: Skip desktop window tests if device creation fails. - approved

[Zebediah Figura (＠zfigura)]

This merge request was approved by Zebediah Figura.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5258

[PATCH 0/1] MR5258: d3d9/tests: Skip desktop window tests if device creation fails. - approved

[Jan Sikorski (＠jsikorski)]

This merge request was approved by Jan Sikorski.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5258