lmi
[Top][All Lists]
Advanced

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

Re: [lmi] wx_test_default_update.cpp


From: Vadim Zeitlin
Subject: Re: [lmi] wx_test_default_update.cpp
Date: Wed, 4 Mar 2015 14:31:48 +0100

On Fri, 23 Jan 2015 19:24:27 +0000 Greg Chicares <address@hidden> wrote:

GC> Let's do this:
GC> (1) make sure "UseDOB" is checked (check it if it isn't); then
GC> (2) set "DateOfBirth" to my birthdate, which you have divined.

 Hello,

 The first attached patch does this.

GC> That's slightly more complicated than we might have hoped,

 If you look at the patch, you can see that it's indeed rather more
complicated than the original version using "MEC avoidance" field because
wxDatePickerCtrl is so difficult to work with from the keyboard portably.
I spent some trying to find some reasonable (i.e. not horribly complicated
and at least somewhat generic) way of driving it from keyboard which would
work with both MSW and GTK and in different locales, but finally abandoned
this approach because I just couldn't do it. So the test now just stuffs
the date directly into the control and then uses some voodoo to make it
work with MvcController update mechanism. It looks a bit horrible in the
code but does work well in practice and I'm certain that it's still much
better than the alternatives.

 Another small complication is that the "UseDOB" field, while being present
in all skins, is not represented by the same control in all of them.
Currently either a check or radio box (with 2 elements) can be used and I
only handle these cases -- even if it could be also represented by a choice
(again, with 2 elements) or a toggle button, it seems unlikely in practice
and if this does happen, the test would start failing and would be easy
enough to fix. Just for the record, I ran this test with skin.xrc (which
uses a wxCheckBox) and skin_coli_boli.xrc (which uses a wxRadioBox) and it
passes with both of them.


GC> >  Speaking of safety, the test specification doesn't say anything about
GC> > preserving the original file, but should the test really modify it?
GC>
GC> Yes, really.

 Please notice that as a consequence of this decision, the test is "run
once": i.e. while it runs successfully when it's launched the first time,
it will fail, because there will be no changes to save in the defaults
file, if it is run again. This seems not ideal to me, e.g. I could imagine
a situation in which some other test fails because of some transient
condition and the test suite needs to be run again -- but not this test
will start (and keep) failing, preventing it from running to the
completion. Of course, this can always be resolved by reinstalling lmi from
scratch, and maybe the distribution tests are only supposed to be run in
such pristine environment. But if not, we could avoid this problem by e.g.
using some other fixed date for the DateOfBirth if it already has the value
from (2) or maybe really toggling (instead of always enabling) UseDOB
field, please let me know if you'd like me to propose patches doing this.


On Wed, 10 Dec 2014 21:28:40 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2014-12-10 21:17, Greg Chicares wrote:
GC> > I've rewritten the comments in this file, but the code already
GC> > does exactly what they say.
GC> 
GC> Oops, I forgot to document one thing that the code doesn't do yet.
GC> Vadim, would you please include the change below in a patch that
GC> addresses what it says?
GC> 
GC> Index: wx_test_default_update.cpp
GC> ===================================================================
GC> --- wx_test_default_update.cpp      (revision 6059)
GC> +++ wx_test_default_update.cpp      (working copy)
GC> @@ -39,6 +39,8 @@
GC> 
GC>  /// Make sure the default input file can be opened, modified, and saved.
GC>  ///
GC> +/// Run this test only if the '--distribution' option is given.
GC> +///

 And the second, much more trivial patch, implements the change above.

 As usual, please let me know if you have any questions about these patches
and thanks in advance for applying them,
VZ

Attachment: 0001-Change-the-Date-of-Birth-field-in-the-default-update.patch
Description: Text document

Attachment: 0002-Only-run-the-default-update-test-with-distribution-o.patch
Description: Text document


reply via email to

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