[PATCH 2/2 v3] user32: Add more dialog creation message tests

[Dmitry Timoshkov]

Piotr Caban <piotr at codeweavers.com> wrote:

> >> On 02/16/16 14:08, Dmitry Timoshkov wrote:
> >>> It's getting better, but again doesn't show a lot of details. Please add
> >>> parameters for most of the recorded messages in the sequence, in particular
> >>> EM_SETSEL and WM_GETDLGCODE are the mandatory ones. Also, in order to see
> >>> the origin of EM_SETSEL I'd suggest to call DefDlgProc() manually and use
> >>> defwndproc_counter around it together with the defwinproc flag.
> >> DefDlgProc can't be called manually in this case. There are already some
> >> comments about it in the tests.
> > TestDlgProcA() calls DefDlgProc() this way without any problem.
> It's because this test is using CreateWindow function family instead of 
> CreateDialog*.

Then please create a window for testing using a way that is appropriate
for your testing purposes and that allows using DefDlgProc().

> >   Even of
> > calling DefDlgProc() is not desirable from inside of a dialog procedure
> > for some reason, then the reason should be explained,
> As I said it's already explained in the tests. Look e.g. on comment in 
> test_dlg_proc function.

Since I've created that function I know why that comment has been added,
and that comment has nothing to do with the way of testing I described
above.

> >   and it's always
> > possible to call DefDlgProc() outside of dialog proc directly and record
> > the produced messages, there are examples in the tests how to do that.
> But it doesn't prove anything. It's something like: it's not possible to 
> test if something is done in this place so lets test everything else to 
> make sure it's not done there. Such tests can be added to test 
> DefDlgProc, not to show that my patch is valid.
> I'm not planning to add more tests related to this code. I think that 
> this change is quite obvious and already well tested.

Many of the tests in msg.c and win.c are created by me, I know the related
code in user32 pretty well, and based on my knowledge I can't agree that
your change is obvious, at least it's absolutely not obvious to me. Focus
handling is very fragile part of user32 code, there are applications that
are very sensitive to any change in that area, and it's very easy to break
them. Please take time to create a more convincing set of tests.

-- 
Dmitry.