[ISSUE #8262] [Enhancement] Add tests and harden UtilAll.isItTimeToDo#10409
[ISSUE #8262] [Enhancement] Add tests and harden UtilAll.isItTimeToDo#10409ChaoXoX wants to merge 1 commit into
Conversation
RongtongJin
left a comment
RongtongJin
left a comment
There was a problem hiding this comment.
Thanks for the patch. I left two review comments around the behavior change and test stability.
| if (hour == nowHour) { | ||
| return true; | ||
| } | ||
| } catch (NumberFormatException ignored) { |
There was a problem hiding this comment.
Could we avoid silently ignoring invalid when tokens here, or at least log a warning when a token cannot be parsed or is outside the 0-23 range? This helper is used by scheduled maintenance paths such as commitlog cleanup and topic queue mapping cleanup. If deleteWhen or a similar schedule is misconfigured, returning false forever would silently disable the task and make the operator miss the configuration problem. The previous behavior failed loudly; changing that to silent ignore seems risky without some visibility.
| assertTrue(UtilAll.isItTimeToDo("foo; " + currentHour + " ; 25")); | ||
| assertTrue(UtilAll.isItTimeToDo(" " + currentHour + " ")); | ||
|
|
||
| assertFalse(UtilAll.isItTimeToDo(null)); |
There was a problem hiding this comment.
This test depends on the hour read here matching the hour read inside UtilAll.isItTimeToDo. It can theoretically become flaky if the test crosses an hour boundary between the two calls. Could we make the positive case independent of wall-clock timing, for example by passing all valid hours (0;1;...;23) and asserting it returns true, while keeping the invalid-token cases separate?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #10409 +/- ##
=============================================
- Coverage 48.99% 47.94% -1.06%
+ Complexity 13459 13258 -201
=============================================
Files 1375 1376 +1
Lines 100394 100546 +152
Branches 12964 12985 +21
=============================================
- Hits 49188 48202 -986
- Misses 45217 46401 +1184
+ Partials 5989 5943 -46 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
Before Creating the Enhancement Request
I have confirmed that this should be classified as an enhancement rather than a bug/feature.
Summary
为 UtilAll.isItTimeToDo 增加边界输入的容错处理，并补充对应单测，提高 common 模块覆盖率。
Motivation
当前方法对 null、空白和非法数字输入会抛 NumberFormatException，容易影响定时任务判断流程。补充容错逻辑与单测能提高健壮性并提升覆盖率。
Describe the Solution You'd Like
UtilAll.isItTimeToDo 对 null/空白/非法 token 安全忽略，跳过越界小时值。
新增单测覆盖：
当前小时命中
含非法 token 的混合输入
null/空串/空白/非法数字/越界小时
Describe Alternatives You've Considered
暂无
Additional Context
关联 #8262（提升项目整体测试覆盖率）。