Skip to content

Commit 8502a95

Browse files
murrantmurrant
authored
Fix time intervals sometimes being wrong (librenms#16995)
* Fix time interval Add the context of now to the interval to get accurate diffs fixes: librenms#16690 librenms#16987 * Apply fixes from StyleCI * remove if * Move parseAt to new unit test --------- Co-authored-by: Tony Murray <murrant@users.noreply.github.com>
1 parent e917f15 commit 8502a95

File tree

3 files changed

+71
-38
lines changed

3 files changed

+71
-38
lines changed

‎LibreNMS/Util/Time.php‎

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,8 @@
2525

2626
namespace LibreNMS\Util;
2727

28-
use Carbon\Carbon;
2928
use Carbon\CarbonInterface;
30-
use Carbon\CarbonInterval;
29+
use Illuminate\Support\Carbon;
3130

3231
class Time
3332
{
@@ -62,23 +61,12 @@ public static function formatInterval(?int $seconds, bool $short = false, ?int $
6261
return '';
6362
}
6463

65-
$parts = $parts ?? ($short ? 3 : -1);
66-
6764
try {
68-
// handle negative seconds correctly
69-
if ($seconds < 0) {
70-
return CarbonInterval::seconds($seconds)->invert()->cascade()->forHumans([
71-
'syntax' => CarbonInterface::DIFF_RELATIVE_TO_NOW,
72-
'parts' => $parts,
73-
'short' => $short,
74-
]);
75-
}
76-
77-
return CarbonInterval::seconds($seconds)->cascade()->forHumans([
78-
'syntax' => CarbonInterface::DIFF_ABSOLUTE,
79-
'parts' => $parts,
80-
'short' => $short,
81-
]);
65+
return Carbon::now()->subSeconds(abs($seconds))->diffForHumans(
66+
syntax: $seconds < 0 ? CarbonInterface::DIFF_RELATIVE_TO_NOW : CarbonInterface::DIFF_ABSOLUTE,
67+
short: $short,
68+
parts: $parts ?? ($short ? 3 : 4),
69+
);
8270
} catch (\Exception) {
8371
return '';
8472
}

‎tests/FunctionsTest.php‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
use LibreNMS\Enum\IntegerType;
3030
use LibreNMS\Util\Number;
3131
use LibreNMS\Util\StringHelpers;
32-
use LibreNMS\Util\Time;
3332

3433
class FunctionsTest extends TestCase
3534
{
@@ -98,25 +97,6 @@ public function testDynamicDiscoveryGetValue(): void
9897
$this->assertSame('BBQ', YamlDiscovery::getValueFromData('doubletable', 13, $data, $pre_cache));
9998
}
10099

101-
public function testParseAtTime(): void
102-
{
103-
$this->assertEquals(time(), Time::parseAt('now'), 'now did not match');
104-
$this->assertEquals(time() + 180, Time::parseAt('+3m'), '+3m did not match');
105-
$this->assertEquals(time() + 7200, Time::parseAt('+2h'), '+2h did not match');
106-
$this->assertEquals(time() + 172800, Time::parseAt('+2d'), '+2d did not match');
107-
$this->assertEquals(time() + 63115200, Time::parseAt('+2y'), '+2y did not match');
108-
$this->assertEquals(time() - 180, Time::parseAt('-3m'), '-3m did not match');
109-
$this->assertEquals(time() - 7200, Time::parseAt('-2h'), '-2h did not match');
110-
$this->assertEquals(time() - 172800, Time::parseAt('-2d'), '-2d did not match');
111-
$this->assertEquals(time() - 63115200, Time::parseAt('-2y'), '-2y did not match');
112-
$this->assertEquals(429929439, Time::parseAt('429929439'));
113-
$this->assertEquals(212334234, Time::parseAt(212334234));
114-
$this->assertEquals(time() - 43, Time::parseAt('-43'), '-43 did not match');
115-
$this->assertEquals(0, Time::parseAt('invalid'));
116-
$this->assertEquals(606614400, Time::parseAt('March 23 1989 UTC'));
117-
$this->assertEquals(time() + 86400, Time::parseAt('+1 day'));
118-
}
119-
120100
public function testNumberCast()
121101
{
122102
$this->assertSame(-14.3, Number::cast(-14.3));

‎tests/Unit/TimeUtilityTest.php‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
namespace Tests\Unit;
4+
5+
use Illuminate\Support\Carbon;
6+
use LibreNMS\Tests\TestCase;
7+
use LibreNMS\Util\Time;
8+
9+
class TimeUtilityTest extends TestCase
10+
{
11+
/**
12+
* A basic unit test example.
13+
*/
14+
public function testFormatInterval(): void
15+
{
16+
$this->assertSame('', Time::formatInterval(0));
17+
$this->assertSame('', Time::formatInterval(null));
18+
$this->assertSame('1 second', Time::formatInterval(1));
19+
$this->assertSame('3s', Time::formatInterval(3, true));
20+
$this->assertSame('1 minute', Time::formatInterval(60));
21+
$this->assertSame('1 minute ago', Time::formatInterval(-60));
22+
$this->assertSame('1m 1s', Time::formatInterval(61, true));
23+
$this->assertSame('1 hour', Time::formatInterval(60 * 60));
24+
$this->assertSame('1 day', Time::formatInterval(24 * 60 * 60));
25+
$this->assertSame('2 weeks 3 days 24 minutes 16 seconds', Time::formatInterval(17 * 24 * 60 * 60 + 1456));
26+
$this->assertSame('2 weeks 3 days', Time::formatInterval(17 * 24 * 60 * 60 + 1456, parts: 2));
27+
28+
// different months could change this
29+
$this->travelTo(Carbon::createFromTimestamp(30042), function () {
30+
$this->assertSame('1 month 1 week 2 days 24 minutes', Time::formatInterval(39 * 24 * 60 * 60 + 1456));
31+
$this->assertSame('1mo 1w 2d 24m 16s', Time::formatInterval(39 * 24 * 60 * 60 + 1456, true, 5));
32+
});
33+
34+
// calculate if there is a leap year (could freeze time, try this instead)
35+
if (Carbon::createFromDate(Carbon::now()->year, 2, 28)->isPast()) {
36+
$days = Carbon::now()->isLeapYear() ? 366 : 365;
37+
} else {
38+
$days = Carbon::now()->subYear()->isLeapYear() ? 366 : 365;
39+
}
40+
41+
$this->assertSame('1 year', Time::formatInterval($days * 24 * 60 * 60));
42+
$this->assertSame('1 year ago', Time::formatInterval(-$days * 24 * 60 * 60));
43+
44+
$this->assertSame('4 years', Time::formatInterval(1461 * 24 * 60 * 60));
45+
}
46+
47+
public function testParseAtTime(): void
48+
{
49+
$this->assertEquals(time(), Time::parseAt('now'), 'now did not match');
50+
$this->assertEquals(time() + 180, Time::parseAt('+3m'), '+3m did not match');
51+
$this->assertEquals(time() + 7200, Time::parseAt('+2h'), '+2h did not match');
52+
$this->assertEquals(time() + 172800, Time::parseAt('+2d'), '+2d did not match');
53+
$this->assertEquals(time() + 63115200, Time::parseAt('+2y'), '+2y did not match');
54+
$this->assertEquals(time() - 180, Time::parseAt('-3m'), '-3m did not match');
55+
$this->assertEquals(time() - 7200, Time::parseAt('-2h'), '-2h did not match');
56+
$this->assertEquals(time() - 172800, Time::parseAt('-2d'), '-2d did not match');
57+
$this->assertEquals(time() - 63115200, Time::parseAt('-2y'), '-2y did not match');
58+
$this->assertEquals(429929439, Time::parseAt('429929439'));
59+
$this->assertEquals(212334234, Time::parseAt(212334234));
60+
$this->assertEquals(time() - 43, Time::parseAt('-43'), '-43 did not match');
61+
$this->assertEquals(0, Time::parseAt('invalid'));
62+
$this->assertEquals(606614400, Time::parseAt('March 23 1989 UTC'));
63+
$this->assertEquals(time() + 86400, Time::parseAt('+1 day'));
64+
}
65+
}

0 commit comments

Comments
 (0)