lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] wx_test_focus_controller_child() assertion not firing


From: Vadim Zeitlin
Subject: Re: [lmi] wx_test_focus_controller_child() assertion not firing
Date: Fri, 23 Mar 2018 10:45:26 +0100

On Fri, 23 Mar 2018 00:53:20 +0000 Greg Chicares <address@hidden> wrote:

GC> I just pushed a change that sets two input fields in a GUI test.
GC> At first, I made the mistake of specifying the fields by label
GC> rather than by name, e.g.:
[...]
GC> I figured that I should add a test for argument validity to
GC> wx_test_focus_controller_child()...but there's one already:
GC> 
GC> wxWindow* wx_test_focus_controller_child(MvcController& dialog, char const* 
name)
GC> {
GC>     // First find the window anywhere inside the dialog.
GC>     wxWindow* const w = wxWindow::FindWindowByName(name, &dialog);
GC>     LMI_ASSERT_WITH_MSG(w, "window named \"" << name << "\" not found");
GC> 
GC> What happened?

 I used the wrong function in this code. In spite of its name,
FindWindowByName() doesn't search the window by its name or, rather, it
does do this first, but then falls back on searching by its label. This is
the documented behaviour but, as proved by my mistake, can still be
confusing.

 In this particular case, there is a window with the given label and it's
the wxStaticText just before the window with the corresponding name. So it
is found by FindWindowByName() but, of course, its value can't be set in
the expected way.

 The simplest solution is to just use wxWindow::FindWindow() which only
uses the name and doesn't fall back to the label and this is what I did in

                https://github.com/vadz/lmi/pull/77

An alternative solution could have been to check that the returned window
does have the right name and suggest that maybe a label was used by mistake
if it doesn't, but it didn't seem worth doing this to me, hopefully failure
to find the window will be enough on its own. Please let me know if you
disagree and would like me to redo the patch to do it like this.

 Thanks,
VZ


reply via email to

[Prev in Thread] Current Thread [Next in Thread]