lennym/moment-business-time

Infinite while loop occurs with workingDiff

Closed this issue · 9 comments

I did some testing before using your node module to make sure that it would do what I needed. For the most part, it works really well! Thank you for all that you have done with this.

There is one test that it fails on:

it('Checking during only non-business hours', function () {
    function getBusinessHoursDiff(timestamp, timestamp_end, timezone) {
      if (!timezone)
        timezone = '+08:00';
      if (!timestamp_end)
        timestamp_end = moment().toISOString() + timezone;
      return Math.abs(moment(timestamp + timezone).workingDiff(moment(timestamp_end), 'hours'));
    }

    
    return expect(getBusinessHoursDiff('2016-10-16T03:00:00', '2016-10-16T07:00:00', timezone)).to.be.equal(1)  });

This test gets in an infinite while loop in the workingDiff function:

moment.fn.workingDiff = function workingDiff(comparator, unit, detail) {
    ....
    while(from.format('L') !== to.format('L')) {
        console.log("from: " + from.format('L'))
        console.log("to: " + to.format('L'))
        if (unit === 'day') {
            diff--;
            from = from.addWorkingTime(1, 'day');
        } else {
            diff += from.diff(openingTimes(from)[1], unit, true);
            from = openingTimes(from.nextWorkingDay())[0];
        }
    }
    ....
}

I added comments to prove to myself that this was where it was in an infinite while loop.

Also, you might wonder why I added a timezone offset. It was because my working hours were not working right, because it wasn't to my timezone. That could be another issue in and of itself.

Also, this will help to:

moment.updateLocale('en', {
  workinghours: {
    0: null,                      // Sunday
    1: ['06:00:00', '18:00:00'],  // Monday
    2: ['06:00:00', '18:00:00'],  // Tuesday
    3: ['06:00:00', '18:00:00'],  // Wednesday
    4: ['06:00:00', '18:00:00'],  // Thursday
    5: ['06:00:00', '18:00:00'],  // Friday
    6: null                       // Saturday
  },
  holidays: [
    "*-01-01", // New Years
    "*-01-18", // Martin Luther King Jr. Day
    "*-02-15", // President's Day
    "*-03-28", // Spring Holiday
    "*-05-30", // Memorial Day
    "*-07-04", // Independence Day
    "*-09-05", // Labor Day
    "*-10-09", // Columbus Day
    "*-11-24", // Thanksgiving
    "*-11-25", // Thanksgiving
    "*-12-23", // Christmas Break
    "*-12-24", // Christmas Break
    "*-12-25", // Christmas Break
    "*-12-26", // Christmas Break
    "*-12-30"  // New Years
  ]
});

Thanks for opening an issue @danielmhair, and thanks especially for a comprehensive test case.

I'll try to take a look as soon as possible. I suspect it's related to the fact that the diff is zero, and I just didn't anticipate that case when I was writing it because I'm slack.

Its all good. I appreciate your help!

I just realized, I gave you the wrong test case. I have the code updated properly.

Thanks. I've reproduced the issue and created a minimal failing test at 368c992

Hopefully I should be able to get a fix out soon.

Published fix @0.6.1 - it was fairly simple in the end

Thanks for your help @danielmhair

Thank you for the fix!

Hey @lennym thanks for your help! It did fix the infinite while loop, but there is a test still failing:

expect(moment('2016-10-16T03:00:00+8:00').workingDiff(moment('2016-10-16T07:00:00+8:00'), 'hours')).to.be.equal(1); // 1 Hour because work hours starts at 6AM

Given that my business hours and holidays are likewise:

moment.updateLocale('en', {
  workinghours: {
    0: null,                      // Sunday
    1: ['06:00:00', '18:00:00'],  // Monday
    2: ['06:00:00', '18:00:00'],  // Tuesday
    3: ['06:00:00', '18:00:00'],  // Wednesday
    4: ['06:00:00', '18:00:00'],  // Thursday
    5: ['06:00:00', '18:00:00'],  // Friday
    6: null                       // Saturday
  },
  holidays: [
    "*-01-01", // New Years
    "*-01-18", // Martin Luther King Jr. Day
    "*-02-15", // President's Day
    "*-03-28", // Spring Holiday
    "*-05-30", // Memorial Day
    "*-07-04", // Independence Day
    "*-09-05", // Labor Day
    "*-10-09", // Columbus Day
    "*-11-24", // Thanksgiving
    "*-11-25", // Thanksgiving
    "*-12-23", // Christmas Break
    "*-12-24", // Christmas Break
    "*-12-25", // Christmas Break
    "*-12-26", // Christmas Break
    "*-12-30"  // New Years
  ]
});

I can make a new issue if needed.

@danielmhair A new issue would be good thanks. Or even better would be if you could write your failing test in a branch and PR it for me.

Thanks a lot.

Alright. It's done!
#19