Skip to content

PR: Fix TimerObservable(==) comparing delay against itself - #84

Merged
bvdmitri merged 2 commits into
ReactiveBayes:mainfrom
docxology:fix/timerobservable-equality
Aug 10, 2026
Merged

PR: Fix TimerObservable(==) comparing delay against itself#84
bvdmitri merged 2 commits into
ReactiveBayes:mainfrom
docxology:fix/timerobservable-equality

Conversation

@docxology

Copy link
Copy Markdown
Contributor

Closes: issue-01

Summary

getdelay_ms(t1) == getdelay_ms(t1) on the right-hand side of the second operand is a
copy-paste typo; the delay is never compared with t2. Fix the comparison to
getdelay_ms(t2).

Change

src/observable/timer.jl

# before
Base.:(==)(t1::TimerObservable, t2::TimerObservable) =
    getdelay_ms(t1) == getdelay_ms(t1) && getperiod_ms(t1) == getperiod_ms(t2)

# after
Base.:(==)(t1::TimerObservable, t2::TimerObservable) =
    getdelay_ms(t1) == getdelay_ms(t2) && getperiod_ms(t1) == getperiod_ms(t2)

How to verify

using Rocket, Test
@test TimerObservable(100.0, 50.0) != TimerObservable(999.0, 50.0)
@test TimerObservable(100.0, 50.0) == TimerObservable(100.0, 50.0)

Add the first assertion to test/observable/test_observable_timer.jl to lock it in.

Closes #69

docxology and others added 2 commits August 1, 2026 16:11
)

Locks in the fix so that timers differing only in `delay` no longer
compare equal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.42%. Comparing base (9ddfba9) to head (6d77a54).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #84      +/-   ##
==========================================
- Coverage   78.45%   78.42%   -0.03%     
==========================================
  Files         120      120              
  Lines        3829     3829              
==========================================
- Hits         3004     3003       -1     
- Misses        825      826       +1     

☔ 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.

@bvdmitri

Copy link
Copy Markdown
Member

Maintainer note: added a regression test (test/observable/test_observable_timer.jl) asserting that timers differing only in delay are no longer equal. The original fix commit is unchanged. Held for you to merge once CI is green.

@bvdmitri
bvdmitri merged commit b6d023a into ReactiveBayes:main Aug 10, 2026
15 of 16 checks passed
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.

TimerObservable(==) ignores the delay of the right operand (compares delay with itself)

2 participants