Clock: Time Extent, Maker, and Associated Traits

[da2ce7]

TimeExtent is a simple structure that contains base increment (duration), and amount (a multiplier for the base the base increment). The resulting product of the base and the multiplier is the total duration of the time extent.

TimeExtentClock is a helper that generates time extents based upon the increment length and the time of the clock, this clock is specialised according to the test predicate with the DefaultClockExtentMaker type.

These modules will be used by the later connection cookie implementation.

[da2ce7]

Rebased after merge of #81

[mickvandijke]

Where are the comments >:(

[da2ce7]

To do: as per discussion rename from "period" to "interval" terminology.

[josecelano]

I've added more comments.

[da2ce7]

To do: as per discussion rename from "period" to "interval" terminology.

Hello @WarmBeer @josecelano,

I have rethought the period structure, and now I think that I have a naming that is good. I have decided on the name of 'extent', while it is already used in computer science (for a continuous range of blocks on a disk), it is somewhat adoptable, as blocks all have the same size.

By using simple mathematical terms: Base, Multiplier, and Product for Extent Trait Types, and clear labels for the TimeExtent specialization: increment and amount. I hope the code is much more intuitively comprehensible.

I think that this pull request is now ready for review.

[josecelano]

@da2ce7 I've only added a couple of comments.

[josecelano]

@da2ce7 these are the test for the new features:

        PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::fn_checked_duration_from_nanos::it_should_return_a_duration
        PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::fn_checked_duration_from_nanos::it_should_return_tryfrom_int_error
        PASS [   0.008s] torrust-tracker protocol::clock::timeextent::test::make_time_extent_now::it_should_return_a_time_extent
        PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::make_time_extent_now::it_should_return_none
        PASS [   0.286s] torrust-tracker protocol::clock::timeextent::test::make_time_extent_now::it_should_return_tryfrom_int_error
        PASS [   0.285s] torrust-tracker protocol::clock::timeextent::test::make_time_extent_now_after::it_should_return_a_time_extent
        PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::make_time_extent_now_after::it_should_return_none
        PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::make_time_extent_now_after::it_should_return_tryfrom_int_error
        PASS [   0.008s] torrust-tracker protocol::clock::timeextent::test::make_time_extent_now_before::it_should_return_a_time_extent
        PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::make_time_extent_now_before::it_should_return_none
        PASS [   0.008s] torrust-tracker protocol::clock::timeextent::test::time_extent_decrease::it_should_return_an_negitive_overflow
        PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::make_time_extent_now_before::it_should_return_tryfrom_int_error
        PASS [   0.008s] torrust-tracker protocol::clock::timeextent::test::time_extent_decrease::it_should_return_decreased
        PASS [   0.362s] torrust-tracker protocol::clock::timeextent::test::time_extent_from_sec::it_should_make_time_extent
        PASS [   0.370s] torrust-tracker protocol::clock::timeextent::test::time_extent_default::it_should_make_time_extent
        PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::time_extent_increase::it_should_postive_overflow
        PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::time_extent_increase::it_should_return_increased
        PASS [   0.008s] torrust-tracker protocol::clock::timeextent::test::time_extent_total::it_should_return_none
        PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::time_extent_new::it_should_make_time_extent
        PASS [   0.008s] torrust-tracker protocol::clock::timeextent::test::time_extent_total::it_should_return_total
        PASS [   0.008s] torrust-tracker protocol::clock::timeextent::test::time_extent_total::it_should_return_tryfrom_int_error
        PASS [   0.008s] torrust-tracker protocol::clock::timeextent::test::time_extent_total_next::it_should_get_the_time_extent_total_next
        PASS [   0.008s] torrust-tracker protocol::clock::timeextent::test::time_extent_total_next::it_should_return_none

When I'm reviewing or writing tests, I always check that reading this output makes sense without looking at the implementation. For me, in the case of tests, namespaces (mod names) are also important. For instance. In this case:

PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::fn_checked_duration_from_nanos::it_should_return_tryfrom_int_error

I would like to read:

PASS [   0.009s] torrust-tracker protocol::clock::timeextent::test::checked_duration_from_nanos_function::should_fails_when_nanos_are_out_of_the_duration_range

The concrete error is an implementation detail. I should not change the test name is you thrown a different error type. And with the new name I know when the function throws that error.

Same for other tests.

I like reading this test output like a documentation to know what the strcut/function/mod/ does not only to protect from regression bugs.

UPDATE:

And I would use snake case:

PASS [   0.009s] torrust-tracker protocol::clock::time_extent::test::checked_duration_from_nanos_function::should_fails_when_nanos_are_out_of_the_duration_range

Notice time_extent instead of timeextent.

[da2ce7]

@josecelano reworded the last comments to include you as the co-author. :)

[josecelano]

@da2ce7 wonderful!!!

With a little bit of indent and order in the output of the tests, you get this beautiful documentation for the mod:

clock::time_extent::test::
    fn_checked_duration_from_nanos::
        it_should_be_the_same_as_duration_implementation_for_u64_numbers
        it_should_fail_for_numbers_that_are_too_large
        it_should_give_zero_for_zero_input
        it_should_work_for_some_numbers_larger_than_u64

    make_time_extent::
        fn_now::
            it_should_fail_for_zero
            it_should_give_a_time_extent
            it_should_fail_if_amount_exceeds_bounds

        fn_now_after::
            it_should_fail_for_zero
            it_should_fail_if_amount_exceeds_bounds
            it_should_give_a_time_extent

        fn_now_before::
            it_should_fail_for_zero
            it_should_fail_if_amount_exceeds_bounds
            it_should_give_a_time_extent
    
    time_extent::
        fn_increase::
            it_should_increase
            it_should_not_increase_for_zero
            it_should_fail_when_attempting_to_increase_beyond_bounds

        fn_decrease::
            it_should_decrease
            it_should_not_decrease_for_zero
            it_should_fail_when_attempting_to_decrease_beyond_bounds
            
        fn_default::
            it_should_default_initialize_to_zero

        fn_from_sec::
            it_should_make_from_seconds
            it_should_make_empty_for_zero

        fn_new::
            it_should_make_empty_for_zero
            it_should_make_new

        fn_total::
            it_should_be_zero_for_zero
            it_should_fail_when_product_is_too_large
            it_should_fail_when_too_large
            it_should_give_a_total

        fn_total_next::
            it_should_be_zero_for_zero
            it_should_fail_when_product_is_too_large
            it_should_fail_when_too_large
            it_should_give_a_total

We should write a script to format the output like that. Like other testing frameworks.

For example: https://github.com/Nautilus-Cyberneering/git-queue/actions/runs/3094430462/jobs/5007796148#step:4:6

[da2ce7]

ACK e785a7f