[PATCH 0/1] MR855: d2d1/tests: Skip subsequent todo tests for unsupported properties.

[Alexandre Julliard (＠julliard)]

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

[PATCH 1/1] d2d1/tests: Skip subsequent todo tests for unsupported properties.

[Alexandre Julliard]

From: Alexandre Julliard <julliard(a)winehq.org>

This reduces the number of printed todos in order to stay within the 32kB limit.
---
 dlls/d2d1/tests/d2d1.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c
index 5043b8b02b3..08c7bc05224 100644
--- a/dlls/d2d1/tests/d2d1.c
+++ b/dlls/d2d1/tests/d2d1.c
@@ -10416,13 +10416,15 @@ static void check_system_properties_(unsigned int line, ID2D1Effect *effect, BOO
         name[0] = 0;
         hr = ID2D1Effect_GetPropertyName(effect, test->index, name, sizeof(name));
         todo_wine_if((is_builtin && (test->type == D2D1_PROPERTY_TYPE_ARRAY || test->type == D2D1_PROPERTY_TYPE_STRING)))
-        {
         ok_(__FILE__, line)(hr == S_OK, "Failed to get property name, hr %#lx\n", hr);
+        if (hr == D2DERR_INVALID_PROPERTY)
+        {
+            winetest_pop_context();
+            continue;
+        }
         ok_(__FILE__, line)(!wcscmp(name, test->name), "Got unexpected property name %s, expected %s.\n",
                 debugstr_w(name), debugstr_w(test->name));
-        }
 
-        type = D2D1_PROPERTY_TYPE_UNKNOWN;
         type = ID2D1Effect_GetType(effect, test->index);
         todo_wine_if((is_builtin && (test->type == D2D1_PROPERTY_TYPE_ARRAY || test->type == D2D1_PROPERTY_TYPE_STRING)))
         ok_(__FILE__, line)(type == test->type, "Got unexpected property type %#x, expected %#x.\n",
@@ -11743,6 +11745,12 @@ static void test_effect_2d_affine(BOOL d3d11)
                 D2D1_PROPERTY_TYPE_MATRIX_3X2, (const BYTE *)test->matrix, sizeof(*test->matrix));
         todo_wine
         ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr);
+        if (hr == D2DERR_INVALID_PROPERTY)
+        {
+            ID2D1Bitmap1_Release(bitmap);
+            winetest_pop_context();
+            continue;
+        }
         ID2D1Effect_GetOutput(effect, &output);
 
         ID2D1DeviceContext_GetImageLocalBounds(context, output, &output_bounds);
@@ -11858,6 +11866,12 @@ static void test_effect_crop(BOOL d3d11)
                 (const BYTE *)&test->crop_rect, sizeof(test->crop_rect));
         todo_wine
         ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr);
+        if (hr == D2DERR_INVALID_PROPERTY)
+        {
+            ID2D1Bitmap1_Release(bitmap);
+            winetest_pop_context();
+            continue;
+        }
         ID2D1Effect_GetOutput(effect, &output);
 
         set_rect(&output_bounds, -1.0f, -1.0f, -1.0f, -1.0f);
-- 
GitLab

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

[PATCH 1/1] d2d1/tests: Skip subsequent todo tests for unsupported properties.

[Zebediah Figura (she/her)]

On 9/15/22 06:01, Alexandre Julliard wrote:
> From: Alexandre Julliard <julliard(a)winehq.org>
>
> This reduces the number of printed todos in order to stay within the 32kB limit.

I can't help but wonder, is it useful for the automated test runs to 
actually print all of the todos?

[PATCH 0/1] MR855: d2d1/tests: Skip subsequent todo tests for unsupported properties. - approved

[Nikolay Sivov (＠nsivov)]

This merge request was approved by Nikolay Sivov.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/855

[Alexandre Julliard (＠julliard)]

> I can't help but wonder, is it useful for the automated test runs to actually print all of the todos?

Good question. I feel that there's some value in having them visible by default, otherwise I'm afraid nobody is ever going to look at them. I at least have fixed a number of todos simply because I noticed them while looking for some other error.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/855#note_8572

[Zebediah Figura]

On 9/15/22 14:01, Alexandre Julliard (@julliard) wrote:
> > I can't help but wonder, is it useful for the automated test runs to actually print all of the todos?
>
> Good question. I feel that there's some value in having them visible by default, otherwise I'm afraid nobody is ever going to look at them. I at least have fixed a number of todos simply because I noticed them while looking for some other error.

I'd definitely agree that they should be visible by default.

I suppose one reason that it could be useful to have them visible in 
winetest output is that it allows us to see what the "wrong" value is 
(in cases where that isn't obvious, or depends on the machine). On 
reflection I've looked at winetest reports in the past to discover this.

Maybe it's not worthwhile if it means spending effort on reducing the 
number of todos, though.

[Nikolay Sivov (＠nsivov)]

On Thu Sep 15 19:16:04 2022 +0000, **** wrote:
> Zebediah Figura replied on the mailing list:
> ```
> On 9/15/22 14:01, Alexandre Julliard (@julliard) wrote:
> > > I can't help but wonder, is it useful for the automated test runs to
> > > actually print all of the todos?
> >
> > Good question. I feel that there's some value in having them visible
> > by default, otherwise I'm afraid nobody is ever going to look at them. I
> > at least have fixed a number of todos simply because I noticed them
> > while looking for some other error.
>
> I'd definitely agree that they should be visible by default.
> I suppose one reason that it could be useful to have them visible in 
> winetest output is that it allows us to see what the "wrong" value is 
> (in cases where that isn't obvious, or depends on the machine). On 
> reflection I've looked at winetest reports in the past to discover this.
> Maybe it's not worthwhile if it means spending effort on reducing the 
> number of todos, though.
> ```
> I do look at todos at our tests results page occasionally, to see if I can clear some of them easily when I'm working in the same area.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/855#note_8588