Skip to content

Add immediate firing capability for time alerts and corresponding test #2745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

stastnypremysl
Copy link
Collaborator

@stastnypremysl stastnypremysl commented Jun 30, 2025

Summary

This pull request introduces a new feature to handle cases where a time alert is set for the exact current time in the TestClock implementation. It ensures that such alerts fire immediately and includes corresponding unit tests to validate this behavior.

Bug fix: Immediate Firing for Current Time Alerts

  • crates/common/src/clock.rs: Modified the TestTimer::new invocation to include a new fire_immediately parameter, which is set to true when the alert time equals the current time. This ensures immediate firing of the alert.

Type of change

  • Bug fix (non-breaking)
  • New feature (non-breaking)
  • Breaking change (impacts existing behavior)
  • Documentation update
  • Maintenance / chore

Release notes

  • I added a concise entry to RELEASES.md that follows the existing conventions (when applicable)

Testing

Ensure new or changed logic is covered by tests.

  • Affected code paths are already covered by the test suite
  • I added/updated tests to cover new or changed logic

Testing Enhancements

  • crates/common/src/clock.rs: Added a new test case, test_set_time_alert_when_alert_time_equals_current_time, to verify that alerts set for the current time fire immediately. This test checks the timer count, advances the clock, and asserts the correct event behavior.# Pull Request

@Copilot Copilot AI review requested due to automatic review settings June 30, 2025 22:45
@CLAassistant
Copy link

CLAassistant commented Jun 30, 2025

CLA assistant check
All committers have signed the CLA.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for firing time alerts immediately when the alert time matches the current time and includes a unit test to validate this behavior.

  • Added a fire_immediately flag in TestTimer::new when alert_time_ns == ts_now
  • Updated the TestTimer instantiation to pass the new flag
  • Introduced test_set_time_alert_when_alert_time_equals_current_time to verify immediate firing

@stastnypremysl stastnypremysl force-pushed the clock-alert-immidiate-firing-capability branch from 177b4ec to b6292d1 Compare June 30, 2025 22:50
Copy link
Member

@cjdsellers cjdsellers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @stastnypremysl

@cjdsellers cjdsellers merged commit 20a80f2 into nautechsystems:develop Jul 1, 2025
13 checks passed
@stastnypremysl stastnypremysl deleted the clock-alert-immidiate-firing-capability branch July 1, 2025 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants