I disagree. It's interesting to see such behavior:
lww = new LWWSet<>(map);
lww = lww.remove(25);
Assert.assertEquals(lww, new LWWSet<>(25)); // 25 is still here
We have some set, remove from it and still have this object. Reason of this -
that there is time from the future in set. It's possible scenario, for example,
we got some LWWSet from the other node (which have clocks ahead), removed and
still have object in existence.
I have such case, as you described, also in tests.
Why not to keep FakeTime tests in suit? If I read this test, it would show a
lot of details for me.
26.06.2017, 04:29, "Edward Capriolo" <[email protected]>:
> I assume that for the purposes of this test the only thing that matters is
> testing the branches for coverage.
>
> Timestamp(1,2)
> Timestamp(2,1)
> Timestamp(1,1)
>
> Having a value come from a real of imaginary clock does not matter. The
> only thing that matters is the test documenting what happens in each case.
>
> On Sun, Jun 25, 2017 at 8:37 PM, Русак Максим <[email protected]> wrote:
>
>> You can check this: https://github.com/apache/incubator-gossip/pull/56
>> I changed +10000 to Long.MAX_VALUE. It can't fail
>>
>> 26.06.2017, 03:28, "Русак Максим" <[email protected]>:
>> > Ok, few minutes and I'll do them right
>> >
>> > 26.06.2017, 03:14, "Edward Capriolo" <[email protected]>:
>> >> The tests are flakey on my machine thry fail every other time in
>> current
>> >> state.
>> >>
>> >> On Sunday, June 25, 2017, Русак Максим <[email protected]> wrote:
>> >>
>> >>> Why call clock.nanotime() once?
>> >>> If you want to make tests more deterministic and don't rely on
>> assumption
>> >>> that between two commands there will be less time than 100000
>> nanoseconds.
>> >>> Then it's good intention, I think it makes tests less intuitive, but
>> it's
>> >>> good.
>> >>> But your changes don't achieve this goal. For example:
>> >>>
>> >>> map.put(25, new LWWSet.Timestamps(nano + 100000 + 100000, 0));
>> >>> lww = new LWWSet<>(map);
>> >>> lww = lww.remove(25);
>> >>> Assert.assertEquals(lww, new LWWSet<>(25));
>> >>>
>> >>> lww.remove get clock.nanotime() so you rely that nano + 100000 +
>> 100000 >=
>> >>> clock.nanotime() it isn't fair.
>> >>
>> >> --
>> >> Sorry this was sent from mobile. Will do less grammar and spell check
>> than
>> >> usual.
