[Math] "RandomGeneratorAbstractTest" is flawed

[Gilles <gi...@harfang.homelinux.org>]

Hi.

There are several problems.

There is a "protected" field ("generator").

There is a no-arg constructor that calls a overridable method
("makeGenerator()").  But the method is thus is called *before*
the subclass's constructor has run.

That method initializes the "generator" field.  If the object
returned by "makeGenerator()" needs initialization (performed by
its constructor), the returned instance will be invalid or "null"
or the call might just fail (raising an exception) in the attempt
to construct a generator instance with missing data.

This invalid object is then passed on to the constructor of
"RandomDataGenerator".

But "RandomDataGenerator" is a smart class: if the RNG is null,
it will create one and all the tests will still work; but they
won't use have used the RNG supposedly under test.

There are "@Before"-annotated methods, but it is not clear that
they are necessary.  In "RandomGeneratorAbstractTest", it partly
duplicates initialization performed by the constructor.

A number of test methods defined in "RandomDataGeneratorTest"
are overridden in "RandomGeneratorAbstractTest" in order to do
nothing; yet they are counted as "passed" in Junit's output.

Unit test "testSeeding" rely on an undefined behaviour in that
"makeGenerator()" will return a different instance at each call.

Reusing the same "generator" instance in several test is
deceiving, as Junit instantiates the class for each "@Test"
method.

All in all, I think that:
1. The "protected" instance variables should be abandoned.
2. The testing for "RandomDataGenerator" should be separate
    from the testing of the RNGs.


Gilles


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org

Re: [Math] "RandomGeneratorAbstractTest" is flawed

[Phil Steitz <ph...@gmail.com>]

On 1/13/16 5:46 PM, Gilles wrote:
> Hi.
>
> There are several problems.
>
> There is a "protected" field ("generator").
>
> There is a no-arg constructor that calls a overridable method
> ("makeGenerator()").  But the method is thus is called *before*
> the subclass's constructor has run.
>
> That method initializes the "generator" field.  If the object
> returned by "makeGenerator()" needs initialization (performed by
> its constructor), the returned instance will be invalid or "null"
> or the call might just fail (raising an exception) in the attempt
> to construct a generator instance with missing data.
>
> This invalid object is then passed on to the constructor of
> "RandomDataGenerator".
>
> But "RandomDataGenerator" is a smart class: if the RNG is null,
> it will create one and all the tests will still work; but they
> won't use have used the RNG supposedly under test.
>
> There are "@Before"-annotated methods, but it is not clear that
> they are necessary.  In "RandomGeneratorAbstractTest", it partly
> duplicates initialization performed by the constructor.
>
> A number of test methods defined in "RandomDataGeneratorTest"
> are overridden in "RandomGeneratorAbstractTest" in order to do
> nothing; yet they are counted as "passed" in Junit's output.
>
> Unit test "testSeeding" rely on an undefined behaviour in that
> "makeGenerator()" will return a different instance at each call.
>
> Reusing the same "generator" instance in several test is
> deceiving, as Junit instantiates the class for each "@Test"
> method.
>
> All in all, I think that:
> 1. The "protected" instance variables should be abandoned.
+1
> 2. The testing for "RandomDataGenerator" should be separate
>    from the testing of the RNGs.

+1

The key thing to make sure of is that the tests use fixed seeds. 
Otherwise, we will see lots of transient failures.

Phil
>
>
> Gilles
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org