[PATCH v2] gdi32/tests: MoveToEx() does not return the old point.

[Francois Gouget]

On Mon, 25 Nov 2019, Huw Davies wrote:
[...]
> > @@ -2262,16 +2262,15 @@ static void test_mf_Graphics(void)
> >      ok( ret, "MoveToEx error %d.\n", GetLastError());
> >      ret = LineTo(hdcMetafile, 2, 2);
> >      ok( ret, "LineTo error %d.\n", GetLastError());
> > +    oldpoint.x = oldpoint.y = 11235;
> >      ret = MoveToEx(hdcMetafile, 1, 1, &oldpoint);
> >      ok( ret, "MoveToEx error %d.\n", GetLastError());
> > -
> > -/* oldpoint gets garbage under Win XP, so the following test would
> > - * work under Wine but fails under Windows:
> > - *
> > - *   ok((oldpoint.x == 2) && (oldpoint.y == 2),
> > - *       "MoveToEx: (x, y) = (%ld, %ld), should be (2, 2).\n",
> > - *       oldpoint.x, oldpoint.y);
> > - */
> > +    /* Windows 9x and older may have returned oldpoint as per the
> > +     * documentation. But Windows >= XP does not.
> > +     */
> > +    todo_wine ok(broken(oldpoint.x == 11235 && oldpoint.y == 11235),
> > +       "MoveToEx: oldpoint = (%d, %d), should be (11235, 11235).\n",
> > +       oldpoint.x, oldpoint.y);
> >  
> >      ret = Ellipse(hdcMetafile, 0, 0, 2, 2);
> >      ok( ret, "Ellipse error %d.\n", GetLastError());
> 
> I don't think this makes much sense - the condition won't be tested under
> Wine because of the broken().

Actually broken() only returns true on Windows (my commit message was 
misleading if not flat out wrong on that count). On Wine it always 
returns false. This is why the todo_wine is needed.

> It seems unlikely an app will ever depend on this, so leaving the 
> original comment there is probably the right thing to do.

I really don't like commented out code.
* It bitrots easily.
* One has to run the code to get any information out of it, and that 
  supposes you have the right Windows version available. For instance 
  it's pretty hard to know how Windows 2000 behaves now. A simple 
  non-code comment may have captured that information back in the day.
* The non-code part of this particular comment is wrong: Windows does 
  not return garbage; it leaves the point unmodified.

Windows apps are one case where this could be an issue. The other is 
running a Wine application or dll that depends on this behavior on 
Windows. Fortunately none of the Wine programs depends on this and for 
the dlls it's just comdlg32, riched20, user32 and uxtheme (I'd say 
unlikely for each).

So I can understand not wanting to change the behavior of Wine and in 
that case having a todo_wine we don't intend to fix is a problem. So 
then I would propose this:


    ok((oldpoint.x == 2 && oldpoint.y == 2) ||
       broken(oldpoint.x == 11235 && oldpoint.y == 11235),
       "MoveToEx: oldpoint = (%d, %d), should be (2, 2).\n",
       oldpoint.x, oldpoint.y);


I'll resubmit with this option fleshed out.

-- 
Francois Gouget <fgouget at codeweavers.com>