Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time generator with microseconds #63

Merged
merged 18 commits into from
Dec 26, 2015

Conversation

razorcd
Copy link

@razorcd razorcd commented Nov 17, 2015

No description provided.

end

def forward(seconds= 60)
from = ::Time.now
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entropy leak in the determinism test failure is because Time.now will not return the same result on successive runs with the same seed. If Time.now is used, the user should pass it as an argument. This way, the gem always returns the same value on the same input with the same seed.

@razorcd razorcd changed the title [WIP] Time generator with microseconds Time generator with microseconds Nov 18, 2015
def random_time(from, to)
diff_time = to - from
dif_in_nsec = (diff_time * 1_000_000_000).to_i
rand_in_nsec = FactoryHelper::Config.random.rand(dif_in_nsec-1) + 1 # avoids 0
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to avoid using FactoryHelper::Config.random.rand here is a not so nice alternative:

      def randomize value
        result = value+1
        while result >= value   # needed because if value = 12_555_555, the block randomizes up to 12_999_999
          thousands = value.to_s.length/3
          remain = value / 1000**thousands
          result_arr = [(0..remain).to_a.sample(:random => FactoryHelper::Config.random) * (1000**thousands)]
          thousands.times do |index|
            result_arr << (0..1000).to_a.sample(:random => FactoryHelper::Config.random) * (1000**(thousands-index-1))
          end
          result = result_arr.inject(&:+)
        end
        result
      end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why avoid the existing randomizer?

100.times do
random_time = @tester.forward(now, 60)
assert random_time > now, "Expected > \"#{now}\", but got #{random_time}"
assert random_time < a_minute_later, "Expected < \"#{now}\", but got #{random_time}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Expected < #{a_minute_later},..."

@NikitaAvvakumov
Copy link
Member

Looks good, @razorcd!

rand_in_nsec = FactoryHelper::Config.random.rand(dif_in_nsec-1) + 1 # avoids 0
rand_in_sec = rand_in_nsec.to_r / 1_000_000_000
from + rand_in_sec
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not

def random_time_simple(from, to)
  time_diff = to - from
  rand_time = FactoryHelper::Config.random.rand(time_diff - 1) + 1
  from + rand_time
end

5.times do
  puts random_time_simple(Time.now - 1, Time.now + 1).iso8601(7)
end

# => 2015-11-22T00:42:24.0341640+02:00
# => 2015-11-22T00:42:23.7815417+02:00
# => 2015-11-22T00:42:23.9767937+02:00
# => 2015-11-22T00:42:24.1995736+02:00
# => 2015-11-22T00:42:24.2099311+02:00

This seems to address the original problem of Time.between returning whole-second timestamps.

Also, the exclusion of the two boundaries (from & to) via the -1 and +1 in the random_time method appears to be inconsistent with other gem methods that return a random value from a range. e.g. FH::Time & FH::Date do not do this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem here is that - 1 will subtract 1 second and not 1 nanosecond.

But I think this will work:
time_diff = (to - from).round(9) #=> returns seconds with 9 digits
Needs rounding because (to - from = 0.0000000019999999...) because of limited nr of bits in a float.
and
FactoryHelper::Config.random.rand(time_diff - 0.000000001) + 0.000000001

But even so, .rand will generate more digits than we need. (If we add .floor, it will not work with digits and if we add .random(9), it might round to next nanosecond).
It looks simpler but might complicate the logic.

This - 1) + 1 is there just to exclude the defined from and to times at the nanosecond level.
Between means all dates between the from and to and not between and included. So I considered just avoiding from and to from the end result (at nanosecond level only!).

E.g.

from = ::Time.local(2012, 05, 05,  05,  05,  30, 222_222.222)
to = ::Time.local(2012, 05, 05,  05,  05,  30, 222_222.224)
FactoryHelper::Timestamps.between(from, to) #=> always returns 2012-05-05 05:05:30 +0300
FactoryHelper::Timestamps.between(from, to).nsec #=> always returns 222222223
#as expected
# maybe we should add these to the tests too :)

class Timestamps < Base

class << self
def between(from = ::Time.at(0), to = ::Time.local(2050, 12, 31, 23, 23, 59, 999_999.999))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still not sure which default value to use fot to =
Because ruby will accept anything, even Time.local(999999999999999, 12, 31, 23, 23, 59, 999_999.999).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have to pick a time, which is unfortunate but due to not being able to overflow Ruby, I would pick the Unix time at 0b01111111_11111111_11111111_11111111_11111111_11111111_11111111_11111111 aka the max signed 64-bit integer

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes but:

Time.at(0b01111111_11111111_11111111_11111111_11111111_11111111_11111111_11111111) #=> 292277026596-12-04 17:30:07 +0200

it's a good idea but it is not very practical

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps 0b01111111_11111111_11111111_11111111 is the Unix Millennium Bug then.

@razorcd
Copy link
Author

razorcd commented Nov 25, 2015

NOTICE I also responded at your comment. GitHub closes it because it's on outdated code.
Find it here:
#63 (diff)

100.times do
random_time = @tester.backward(now, 60)
assert random_time < now, "Expected < \"#{strftime_nsec now}\", but got #{strftime_nsec random_time}"
assert random_time > a_minute_earlier, "Expected > \"#{strftime_nsec a_minute_earlier}\", but got #{strftime_nsec random_time}"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strftime_nsec will show a more readable error:

Expected > "2015-12-03 17:30:02 8936975250nsec +0200", but got 2015-12-03 17:29:58 6061458480nsec +0200.

@razorcd
Copy link
Author

razorcd commented Dec 22, 2015

should we add this as a Reminder/Notice in the docs?

Any operation with a Time instance at a level smaller than 1 second should be followed by
a rounding because floating point operations are inaccurate.
e.g. (Time.now + 0.000_000_001).round(9)

@b264
Copy link
Member

b264 commented Dec 26, 2015

👍

b264 added a commit that referenced this pull request Dec 26, 2015
@b264 b264 merged commit 6d5cd24 into factory-helper:master Dec 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants