I think the code has been refactored many times since then.

If the code of the tests is really the same I'm okay with deleting the
duplicate method.

On Tue, Jan 10, 2017 at 3:29 PM, Aljoscha Krettek <aljos...@apache.org>
wrote:

> Thanks for looking into this! I think we can put in the fix and remove one
> of the tests, yes.
>
> @Robert What do you think? I think you initially added this test a loooong
> while back.
>
> On Mon, 9 Jan 2017 at 20:11 Alexey Demin <diomi...@gmail.com> wrote:
>
> > Hi,
> >
> > I am trying make small review for slow test and I found small issue:
> >
> > NonReusingReOpenableHashTableITCase
> >
> > testSpillingHashJoinWithMassiveCollisions
> > testSpillingHashJoinWithTwoRecursions
> >
> >
> > for testSpillingHashJoinWithTwoRecursions exist description
> >
> > /*
> >  * This test is basically identical to the
> > "testSpillingHashJoinWithMassiveCollisions" test, only that the number
> >  * of repeated values (causing bucket collisions) are large enough to
> make
> > sure that their target partition no longer
> >  * fits into memory by itself and needs to be repartitioned in the
> > recursion again.
> >  */
> >
> > but he incorrect, because code of both test fully equal,
> > one difference line very similar on bug after refactoring with inserting
> > recordReuse
> >
> > testSpillingHashJoinWithMassiveCollisions
> >  353   while ((record = buildSide.next(record)) != null) {
> >
> > (f51f1b4 19.03.14, 1:17 Aljoscha Krettek* Change MutableObjectIterator to
> > allow immutable objects)
> >
> >
> > Aljoscha, can we remove one test and fix buildSide.next(record) to
> > buildSide.next(recordReuse) ?
> >
> > P.S. I started review because we have a lot of failing test due to cpu
> > time limit
> >
> > Thanks,
> > Alexey
> >
>

Reply via email to