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-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>



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

Reply via email to