aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTeddy Wing2017-05-26 13:08:29 +0200
committerTeddy Wing2017-05-26 13:20:56 +0200
commitcf8a4d557e7ba7a56175be30d2e2c94ea0e1b928 (patch)
tree4d9dad6a38d8e31249ac0e60f5086c4ab48b4120
parent83d3f1d5fce256f90f308a182e1f3b9de518da45 (diff)
downloadchouette-core-cf8a4d557e7ba7a56175be30d2e2c94ea0e1b928.tar.bz2
TimeDuration.exceeds_gap?: Fix duration > 24 hours behaviour
Previously, the `#exceeds_gap?` method would modulo the `later - earlier` duration against 24 hours such that no duration exceeded 24 hours, and the resulting duration being compared was adjusted to fit inside a single day. When we removed the numbers from `exceeds_gap?`, we also removed the modulo operation. I had a feeling that would potentially cause problems, and it turns out it did. Though it still took too much investigation time to realise that the most obvious answer was in fact the correct one. Blame the confusing test failures if I will: Failures: 1) Chouette::VehicleJourney state_update should return errors when validation failed Failure/Error: expect(state['errors'][:vehicle_journey_at_stops].size).to eq 1 NoMethodError: undefined method `[]' for nil:NilClass # ./spec/models/chouette/vehicle_journey_spec.rb:148:in `block (3 levels) in <top (required)>' 2) Chouette::VehicleJourney state_update vehicle_journey_at_stops should return errors when validation failed Failure/Error: expect(item['errors'][:arrival_time].size).to eq 1 NoMethodError: undefined method `[]' for nil:NilClass # ./spec/models/chouette/vehicle_journey_spec.rb:189:in `block (4 levels) in <top (required)>' 3) VehicleJourneyImport.save should not import vehicle_journeys and not create objects when vehicle journey at stops are not in ascendant order Failure/Error: expect(VehicleJourneyImport.new(:route => route, :file => invalid_file_on_vjas_object).save).to be_falsey expected: falsey value got: true # ./spec/models/vehicle_journey_import_spec.rb:90:in `block (3 levels) in <top (required)>' Finished in 4 minutes 20.7 seconds (files took 14.47 seconds to load) 1089 examples, 3 failures, 23 pending Failed examples: rspec ./spec/models/chouette/vehicle_journey_spec.rb:137 # Chouette::VehicleJourney state_update should return errors when validation failed rspec ./spec/models/chouette/vehicle_journey_spec.rb:183 # Chouette::VehicleJourney state_update vehicle_journey_at_stops should return errors when validation failed rspec ./spec/models/vehicle_journey_import_spec.rb:89 # VehicleJourneyImport.save should not import vehicle_journeys and not create objects when vehicle journey at stops are not in ascendant order These three tests failed with my version of `exceeds_gap?`. To correct the failures, add back in the 24-hour adjustment. Put it in a named method that is hopefully clear enough to understand. The second test failure ("vehicle_journey_at_stops should return errors when validation failed") used two times that were the same. I decided to add a test for this because it seemed like an interesting edge case. The test I added didn't fail even for the previous version of `.exceeds_gap?`, but I figure it seems like a useful test anyway. Refs #870
-rw-r--r--lib/time_duration.rb8
-rw-r--r--spec/lib/time_duration_spec.rb14
2 files changed, 21 insertions, 1 deletions
diff --git a/lib/time_duration.rb b/lib/time_duration.rb
index f64c81bdd..12419fdc8 100644
--- a/lib/time_duration.rb
+++ b/lib/time_duration.rb
@@ -9,6 +9,12 @@ module TimeDuration
# Time.now + 2.hours
# )
def self.exceeds_gap?(duration, earlier, later)
- duration < (later - earlier)
+ duration < self.duration_without_24_hour_cycles(later - earlier)
+ end
+
+ private
+
+ def self.duration_without_24_hour_cycles(duration)
+ duration % 24.hours
end
end
diff --git a/spec/lib/time_duration_spec.rb b/spec/lib/time_duration_spec.rb
index 2d63eba76..1cba1f6d5 100644
--- a/spec/lib/time_duration_spec.rb
+++ b/spec/lib/time_duration_spec.rb
@@ -14,6 +14,20 @@ describe TimeDuration do
t2 = Time.now + (4.hours + 1.minutes)
expect(TimeDuration.exceeds_gap?(4.hours, t1, t2)).to be_truthy
end
+
+ it "returns true when `earlier` is later than `later`" do
+ earlier = Time.new(2000, 1, 1, 11, 0, 0, 0)
+ later = Time.new(2000, 1, 1, 12, 0, 0, 0)
+
+ expect(TimeDuration.exceeds_gap?(4.hours, later, earlier)).to be true
+ end
+
+ it "returns false when `earlier` == `later`" do
+ earlier = Time.new(2000, 1, 1, 1, 0, 0, 0)
+ later = earlier
+
+ expect(TimeDuration.exceeds_gap?(4.hours, later, earlier)).to be false
+ end
end
end
end