bokmann / business_time

Support for doing time math in business hours and days
MIT License
1.27k stars 211 forks source link

during_business_hours? returning nil since 0.10.0 #207

Closed MiguelCorti closed 3 years ago

MiguelCorti commented 3 years ago

Using the during_business_hours? method used to return true or false correctly on version 0.9.3.

Since the update to 0.10.0 it started to return nil everytime, for instance, running Time.now.during_business_hours? returns nil. Forcing the version back to 0.9.3 made it work again.

I'm using ruby 2.4.9

Heres an example: image

And forcing a workday config also doesn't work: image

Thanks!

rmm5t commented 3 years ago

Sorry, I cannot reproduce your issue. I tried with both Ruby 2.4.9 and Ruby 2.6.6. Is there something else that might be causing your issue?

$ bin/console 
>> RUBY_VERSION
=> "2.6.6"
>> BusinessTime::VERSION
=> "0.10.0"
>> Time.now.during_business_hours?
=> true
$ bin/console 
>> RUBY_VERSION
=> "2.4.9"
>> BusinessTime::VERSION
=> "0.10.0"
>> Time.now.during_business_hours?
=> true
rmm5t commented 3 years ago

Closing this out until more information is provided.

ducharmemp commented 2 years ago

@rmm5t I think I have a replication available along with the root cause, and I've replicated a test failure in our CI environment and my local machine.

RUBY_VERSION=2.7.4 Testing business time versions 0.9.3, 0.11.0

Test date: Sun, 14 Oct 2018 00:00:00 EDT -04:00 (although a Saturday would work as well)

In Ruby 2, sortedset is part of the standard library, while in Ruby 3, sortedset was removed with a key difference.

In the gem, rbtree is always required https://github.com/knu/sorted_set/blob/master/lib/sorted_set.rb#L7

In the original standard library implementation, rbtree tries to load, but falls back to a normal hash if rbtree is unavailable (it's unavailable on my system if I try to run require "rbtree").

If a normal hash is used, the default value is false https://github.com/ruby/ruby/blob/master/lib/set.rb#L246 but as far as I can tell, there's no such restriction in rbtree. Below is an IRB console session that I captured on both 0.9.3 and 0.11.0

# 0.9.3
BusinessTime::Config.weekdays.instance_variable_get("@hash")
=> {1=>true, 2=>true, 3=>true, 4=>true, 5=>true}
BusinessTime::Config.weekdays.include?(0)
=> false

# 0.11.0
BusinessTime::Config.weekdays.instance_variable_get("@hash")
=> #<RBTree: {1=>true, 2=>true, 3=>true, 4=>true, 5=>true}, default=nil, cmp_proc=nil>
BusinessTime::Config.weekdays.include?(0)
=> nil

ETA: bug report on sorted_set available here: https://github.com/knu/sorted_set/issues/2

Ah I see there was also a PR in this repo that was closed, apologies for the ping! https://github.com/bokmann/business_time/pull/208