You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gossip.apache.org by Русак Максим <ma...@yandex.ru> on 2017/06/25 20:46:14 UTC
Fwd: GOSSIP-90 Why call clock.nanotime() once?
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.
Re: GOSSIP-90 Why call clock.nanotime() once?
Posted by Русак Максим <ma...@yandex.ru>.
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" <ed...@gmail.com>:
> 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, Русак Максим <ma...@yandex.ru> 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, "Русак Максим" <ma...@yandex.ru>:
>> > Ok, few minutes and I'll do them right
>> >
>> > 26.06.2017, 03:14, "Edward Capriolo" <ed...@gmail.com>:
>> >> The tests are flakey on my machine thry fail every other time in
>> current
>> >> state.
>> >>
>> >> On Sunday, June 25, 2017, Русак Максим <ma...@yandex.ru> 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.
Re: GOSSIP-90 Why call clock.nanotime() once?
Posted by Edward Capriolo <ed...@gmail.com>.
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, Русак Максим <ma...@yandex.ru> 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, "Русак Максим" <ma...@yandex.ru>:
> > Ok, few minutes and I'll do them right
> >
> > 26.06.2017, 03:14, "Edward Capriolo" <ed...@gmail.com>:
> >> The tests are flakey on my machine thry fail every other time in
> current
> >> state.
> >>
> >> On Sunday, June 25, 2017, Русак Максим <ma...@yandex.ru> 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.
>
Re: GOSSIP-90 Why call clock.nanotime() once?
Posted by Русак Максим <ma...@yandex.ru>.
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, "Русак Максим" <ma...@yandex.ru>:
> Ok, few minutes and I'll do them right
>
> 26.06.2017, 03:14, "Edward Capriolo" <ed...@gmail.com>:
>> The tests are flakey on my machine thry fail every other time in current
>> state.
>>
>> On Sunday, June 25, 2017, Русак Максим <ma...@yandex.ru> 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.
Re: GOSSIP-90 Why call clock.nanotime() once?
Posted by Русак Максим <ma...@yandex.ru>.
Ok, few minutes and I'll do them right
26.06.2017, 03:14, "Edward Capriolo" <ed...@gmail.com>:
> The tests are flakey on my machine thry fail every other time in current
> state.
>
> On Sunday, June 25, 2017, Русак Максим <ma...@yandex.ru> 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.
Re: GOSSIP-90 Why call clock.nanotime() once?
Posted by Edward Capriolo <ed...@gmail.com>.
The tests are flakey on my machine thry fail every other time in current
state.
On Sunday, June 25, 2017, Русак Максим <ma...@yandex.ru> 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.