[ISSUE #8262] [Enhancement] Add tests and harden UtilAll.isItTimeToDo

[ChaoXoX]

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（提升项目整体测试覆盖率）。

[RongtongJin]

Thanks for the patch. I left two review comments around the behavior change and test stability.

[RongtongJin]

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.

[RongtongJin]

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-commenter]

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.95%. Comparing base (9879968) to head (7341436).
⚠️ Report is 56 commits behind head on develop.

Files with missing lines	Patch %	Lines
.../main/java/org/apache/rocketmq/common/UtilAll.java	83.33%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10409      +/-   ##
=============================================
- Coverage      48.99%   47.95%   -1.05%     
+ Complexity     13459    13282     -177     
=============================================
  Files           1375     1377       +2     
  Lines         100394   100623     +229     
  Branches       12964    12994      +30     
=============================================
- Hits           49188    48251     -937     
- Misses         45217    46432    +1215     
+ Partials        5989     5940      -49     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[oss-sentinel-ai]

Review: Approved ✅

PR: #10409 — [Enhancement] Add tests and harden UtilAll.isItTimeToDo
Type: Enhancement + Test coverage (12 files, +49/-17)

Code Analysis

UtilAll.isItTimeToDo hardening:

• ✅ Null/blank guard at method entry — prevents NPE
• ✅ Try-catch around Integer.parseInt — gracefully handles non-numeric tokens instead of throwing NumberFormatException
• ✅ Hour range validation (0-23) — skips out-of-range values
• ✅ Calendar.HOUR_OF_DAY computed once — minor efficiency improvement

Test coverage:

• ✅ Current hour match (positive case)
• ✅ Mixed input with invalid tokens (foo; <hour> ; 25)
• ✅ Whitespace handling
• ✅ Null, empty, blank, non-numeric, out-of-range (negative cases)

Documentation changes:

• Minor doc file renames and link fixes — unrelated but harmless

Verdict

Clean, well-scoped enhancement that improves robustness of scheduled task timing logic. Test coverage is comprehensive. No concerns.

---

🤖 Automated review by oss-sentinel-ai

[RockteMQ-AI]

Review by github-manager-bot

Summary

Harden UtilAll.isItTimeToDo to silently skip null/blank/invalid/out-of-range hour tokens instead of throwing NumberFormatException, with new unit tests and unrelated doc fixes.

Findings

• [Critical] common/src/test/java/org/apache/rocketmq/common/UtilAllTest.java:156 — The @Test annotation is corrupted: the diff shows @repos/apache_rocketmq/auth/src/test/java/... instead of @Test. This means testIsItTimeToDo will not be recognized as a test method and will never run. The entire test coverage rationale for this PR breaks down.

• [Warning] common/src/main/java/org/apache/rocketmq/common/UtilAll.java:120 — The outer if (whiles.length > 0) guard is now dead code: String.split(";") on a non-blank string always returns an array of length ≥ 1. Not a bug, but misleading.

• [Warning] common/src/test/java/org/apache/rocketmq/common/UtilAllTest.java:157 — The positive test assertTrue(UtilAll.isItTimeToDo(String.valueOf(currentHour))) is time-sensitive and will fail if the hour rolls over between Calendar.getInstance() and isItTimeToDo's own Calendar.getInstance() call (extremely rare, but flaky by design). Consider passing a fixed mock or accepting the tiny risk explicitly.

• [Warning] common/src/test/java/org/apache/rocketmq/common/UtilAllTest.java:158 — assertFalse(UtilAll.isItTimeToDo("99")) is correct for the out-of-range case, but there is no assertFalse for a valid hour that is not the current hour (e.g., (currentHour + 1) % 24). The negative path for a well-formed but non-matching hour is untested.

• [Info] common/src/main/java/org/apache/rocketmq/common/UtilAll.java:130 — The catch block comment ("Ignore invalid hour tokens to avoid breaking scheduled tasks.") violates the project's no-obvious-comment style, but is borderline acceptable given the silently-swallowed exception.

• [Info] Doc renames (Design_LoadBlancing.md → Design_LoadBalancing.md, Design_Trancation.md → Design_Transaction.md, Troubleshoopting.md → Troubleshooting.md) fix long-standing typos; these are welcome cleanups but are unrelated to the stated issue scope.

Suggestions

1. Fix the @Test annotation immediately — the annotation on testIsItTimeToDo is the file path of an unrelated file (@repos/apache_rocketmq/auth/...). This is the blocking issue; the PR's primary value (test coverage) is entirely negated without it.
2. Add a assertFalse(UtilAll.isItTimeToDo(String.valueOf((currentHour + 1) % 24))) case to cover the valid-but-non-matching hour path.
3. Remove the dead if (whiles.length > 0) wrapper, or at minimum flatten the indentation, to keep the method readable.

---

Automated review by github-manager-bot