Round is not working correctly

[schoofseggl]

I think there is an issue with the round function. See this plunker:

https://plnkr.co/edit/RRyKjyroA2ho9wZcO1gn?p=preview

For 7200 sec, the result will be 2 hours. Fine. For 7199 sec the result is now 1 hour for options.largest =1. Shouldn't it be 2 hours?

[EvanHahn]

Sorry to take a few days to respond to this! Could you post a snippet of JavaScript that reproduces this problem?

[schoofseggl]

Have you noticed the plunk above? It's a small angular filter calls the shortEn humanizer (took it from your website)

[EvanHahn]

I have but I didn't see how it reproduced the problem (the largest option didn't appear to be set). Any chance you could do something that isn't Angular-specific?

I'm happy to dig into this further but am busy so it might take me awhile to get to this.

[schoofseggl]

Vanilla plunk:

https://plnkr.co/edit/KYn1iltDBl4viEDQE4Ig

[EvanHahn]

Hmm, that didn't appear to work for me. Could you just show a call to humanizeDuration that breaks for you? It doesn't need to be in a Plunker...something like:

humanizeDuration(7200, {largest: 1, round: true}) // => whatever you get

[schoofseggl]

Arg, forgot to save the plunk. New one: https://plnkr.co/edit/WwL30XZbp8kffm5EN22I?p=preview

For 7200 seconds I get

with round, largest 2: 2 hours with round, largest 1: 2 hours

Which is correct obviously

However 7199 seconds yields

with round, largest 2: 1h 59m (correct) with round, largest 1: 1h

But shouldn't this be rounded to 2h?

[EvanHahn]

Yes, I think you're right.

Any chance you would be able to submit a pull request for this? I am pretty busy and likely won't be able to get to this too soon.

[2bezzat]

@EvanHahn I can help in this if you're still busy

[EvanHahn]

@A-ezzat1997 That'd be great!

[EvanHahn]

@A-ezzat1997 Are you able to make a pull request for this?

[2bezzat]

@EvanHahn I'm sorry I was very busy the last couple of months and the coming months will be so as well, once I can work on it and there's no one already fix it I'll work on it

[EvanHahn]

@A-ezzat1997 Any chance you're able to make a pull request here? (Or anyone else?)

[lharress]

@EvanHahn FYI ~ This bug was squashed in v3.6.1, specifically this commit

[EvanHahn]

@lharress That was committed over 3 years ago, but this this report was from about 1 year ago. Are you sure that commit squashed this bug?

[lharress]

@EvanHahn The initial issue is gone, here's an updated plunk with the latest code from master;

https://next.plnkr.co/edit/I3YBFw1VaZw4MekP

One thing I'd note (which is correct IMO) is that using 7199 with { largest: 2, round: true } gives 2 hours given that 1hr, 59mins and 59 seconds rounded to the nearest minute bumps it up to 2 hours.

Above @schoofseggl mentions;

However 7199 seconds yields with round, largest 2: 1h 59m (correct)

Which was in fact incorrect as well.

[HHK1]

Wrote some tests, I don't think there is any issue anymore, everything is passing.

  it('can handle rounding with the "largest" option without truncating the largest units', () => {
    const h = humanizer({ round: true })
    assert.strictEqual(h(739160000, { largest: 2 }), '1 week, 2 days')
    assert.strictEqual(h(739160000, { largest: 2 }), '1 week, 2 days')
    assert.strictEqual(h(7199000, { largest: 2 }), '2 hours')
    assert.strictEqual(h(7199000, { largest: 3 }), '1 hour, 59 minutes, 59 seconds')
    assert.strictEqual(h(7201000, { largest: 2 }), '2 hours')
    assert.strictEqual(h(7201000, { largest: 3 }), '2 hours, 1 second')
  })

[EvanHahn]

It's been a few years but I'm finally getting around to this.

Could you try this on the latest version of humanize-duration to see if this is still a problem? I think I may have fixed it.

[schoofseggl]

At least for the issue I reported initially I can confirm it's fixed, thanks :)

[EvanHahn]

Great! I'm going to close this issue because I think it's (finally) solved.

Thanks for your patience!