fix(tests): allow same-day rule tunings without updated_date bump - #6681
fix(tests): allow same-day rule tunings without updated_date bump#6681Mikaayenson wants to merge 3 commits into
Conversation
When a modified rule already has metadata.updated_date set to today (UTC), skip requiring a new updated_date hunk in the git diff. Stale dates still fail. Reads updated_date via pytoml.
Mikaayenson
added
bug
There was a problem hiding this comment.
Pull request overview
Updates the rule-modification test logic to allow same-day follow-up rule tunings without requiring an updated_date line change in the PR diff, while still enforcing updated_date bumps when the recorded date is stale (using UTC “today” for CI consistency).
Changes:
- Update
test_rule_change_has_updated_dateto pass whenupdated_dateis already today (UTC) even if the diff doesn’t include anupdated_datehunk. - Treat missing
metadata.updated_dateas a pass rather than forcing a bump. - Parse rule TOML via
pytomland normalizeupdated_datevalues into a UTC date for comparison.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @unittest.skipIf(os.getenv("GITHUB_EVENT_NAME") == "push", "Skipping this test when not running on pull requests.") | ||
| def test_rule_change_has_updated_date(self): | ||
| """Test to ensure modified rules have updated_date field updated.""" | ||
| """Fail when a modified rule lacks an updated_date bump and is not same-day UTC.""" |
| updated_date = metadata["updated_date"] | ||
| if isinstance(updated_date, datetime): | ||
| if updated_date.tzinfo is None: | ||
| updated_date = updated_date.replace(tzinfo=UTC) |
There was a problem hiding this comment.
In this case, other places in the code covert the time to non-UTC so this will still could cause an issue if fixed this way. Since in this repo updated date is required I think it needs to be handled a little differently.
Pulling local time:
- detection_rules/cli_utils.py:62
time.strftime("%Y/%m/%d") - detection_rules/cli_utils.py:271
datetime.date.today() - detection_rules/devtools.py:1010,1165,1758,2026
time.strftime("%Y/%m/%d")
So cases with something like: US/Pacific, Aug 20 18:00 PDT (= Aug 21 01:00 UTC)
I think the simplest way to address this is to just have our functions in cli_utils.py and devtools.py also be timezone aware to UTC (e.g. datetime.now(UTC).strftime("%Y/%m/%d") ), to address the conflict.
patch to fix:
There was a problem hiding this comment.
if this is the case, agreed, maybe a separate PR to add a globally accessible adjustment.
eric-forte-elastic
left a comment
eric-forte-elastic
left a comment
There was a problem hiding this comment.
Main comment, otherwise LGTM 👍
Summary
updated_datehunk in the git diffmetadata.updated_dateas pass; require a bump when the date is staleupdated_dateviapytoml(aligned with endpoint-rules same-day check)detection_rulespackage version2.1.6→2.1.7Test plan
updated_datealready UTC today) does not failtest_rule_change_has_updated_dateupdated_datewith no bump still fails+updated_datein the diff still passes