Re: [lmi] wx_test_input_sequences.cpp

[Vadim Zeitlin]

On Mon, 09 Mar 2015 14:23:50 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2015-03-04 22:35, Vadim Zeitlin wrote:
GC> > On Fri, 12 Dec 2014 15:17:49 +0000 Greg Chicares <address@hidden> wrote:
GC> > 
GC> > GC> ...so whenever an individual sequence-paste test cannot be run because
GC> > GC> of these reasons, I believe we should skip it silently.
GC> > 
GC> >  I've interpreted the word "individual" above as meaning that if one of 
the
GC> > fields doesn't exist in the current skin, we should skip only testing the
GC> > input sequence(s) which is/are supposed to be used with this field, but
GC> > still continue testing the other ones -- and this is what the patch does.
GC> 
GC> Perfect. That's exactly what we want.
GC> 
GC> > Personally I think it's not the best idea because you have no idea at all
GC> > if the tests were run or not, if the entire test were skipped there would
GC> > at least be a mention of it in the output ("skipped (missing field in the
GC> > skin)" or something like this), but currently there is no indication of it
GC> > at all. So if, e.g. "Payment" field is ever renamed, more than half of the
GC> > sequences wouldn't be tested any longer, but the test would still be
GC> > reported as passing. I guess you can always say that you would notice this
GC> > by seeing that the test execution time has suddenly drastically decreased,
GC> > but I can't really convince myself that it's a good idea to rely on 
timings
GC> > to check if the tests were run or not.
GC> 
GC> First, let's recapitulate some background information. Class Input contains
GC> the whole universe of input fields. The several skins contain various 
subsets
GC> of that universe.

 Thanks for writing this, this is probably painfully obvious for you, but I
didn't completely realize before reading your email that Input defined all
the fields.

GC> We'll frequently run the GUI test with 'skin.xrc', so it doesn't matter that
GC> other skins lack some input-sequence fields: as long as 'skin.xrc' has them
GC> all, they'll all be tested.

 This makes total sense now.

GC> But you're asking what happens if we change the name of a field. Right now,
GC> the test would treat that field as though it didn't exist: a test that had
GC> always done something would silently begin to do nothing. I think it's a
GC> good idea to guard against that by making sure the field name is still part
GC> of the universe to which it originally belonged.
...
GC> Sure, that works too:
GC> 
GC>                 // If the field for this input sequence doesn't exist in the
GC> !               // currently used skin at all, then say so, and skip this
GC> !               // particular sequence silently -- but continue testing the
GC> !               // other ones.
GC> !               appropriate_output_stream << "Field " << field << " skipped 
because...";
GC>                 return wxID_CANCEL;
GC> 
GC> Not quite as in-your-face as the preceding idea, but good enough:
GC> I'd be sure to notice if it fails to find a field in 'skin.xrc'.

 I've chosen to use a warning message instead of failing the test
completely, if only to allow finding all out of date fields at once instead
of having to fix them one by one.

 Notice that the attached patch uses wxPrintf() to write to the
"appropriate output stream" and so it logically, even if not physically
(i.e. it can be applied to and would work with the current trunk as is),
depends on the first patch attached to the 20150309T18:52:35 message
(http://lists.nongnu.org/archive/html/lmi/2015-03/msg00026.html) in the
"Error reporting" thread.

 As always, please let me know if you see any problems with this patch and
thanks in advance for applying it,
VZ

0001-Complain-about-unknown-field-names-in-the-input-sequ.patch
Description: Text document