Re: [lmi] [PATCH] New input validation test

[Greg Chicares]

On 2015-03-13 00:26, Vadim Zeitlin wrote:
[...]
> On Thu, 13 Nov 2014 03:28:13 +0000 Greg Chicares <address@hidden> wrote:
[...]
> GC> Here's how we'd like to change that test so that it encompasses
> GC> everything that is at present done manually. First, open an
> GC> '.ill' file that we'll provide to replace "CoiMultiplier.cns".
> 
>  The attached patches rely on the presence of CoiMultiplier.ill file in the
> GUI tests data directory.

IIRC (and I'd better), the idea here is to use a proprietary product
that permits a COI multiplier but requires it to be within a certain
range that varies by password.

> GC> Its parameters will all be defaults, except that we'll choose a
> GC> group executive carveout product.

Good, I did recall correctly.

>  For testing, I used the default ("sample") product, but I changed the
> value of MinInputCoiMult in sample.database to 0.9.

Sure, that's the right way for you to do it locally.

> GC> Then we'll force values (a-j)
> GC> above into the
> GC>   <object class="InputSequenceEntry" name="CurrentCoiMultiplier">
> GC> field, simulating "OK" after each one, and test the outcome
> GC> against the diagnostics that Input::RealizeCurrentCoiMultiplier()
> GC> prescribes.
> 
>  This is exactly what the current patches do. Please notice that the first
> patch in the series is a pure refactoring,

Committed 20150327T0027Z, revision 6152.

> the second one is a trivial new
> class addition

Committed 20150327T0032Z, revision 6153.

> and the only significant patch is the last one, which

Committed 20150327T0041Z, revision 6154.

> updates the input_validation test code and specification. The code is a bit
> long due to the presence of a couple of special cases, but I don't see any
> simple way to improve it, so I commented it to at least explain what is
> going on here instead.
> 
>  If the code complexity bothers you (it does bother me, FWIW), then I'd
> like to note that good part of it comes from the need to deal with (and
> check for) domain_error exception thrown for the negative COI multiplier
> when running with --mellon option. If we could avoid this test (for the
> value "-1"), the modifications to SkeletonTest would be unnecessary and the
> code of the test itself could be also simplified. Of course, this is not a
> good reason to drop/skip this test if it is valuable, but if it is not, it
> might be a good idea.

It feels right to me. I think the code being tested is right to distinguish
domain_error in this case, and I don't mind that the code to test it is
therefore a little more complicated.

Now we're about to add a new product with different restrictions on this
'CurrentCoiMultiplier' field. This unit test will help us guard against
regression errors in this new work.

And now I think I've applied all your 2015 patches except for the census
manager changes (which I must defer) and the wxxrc explicit-size removal
(which I will look into soon (especially because it might help me, with
my high DPI setting) as I need to add some new input fields anyway).