Skip to content
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

Add unit tests for SystemTimer.h #12729

Closed
kpschoedel opened this issue Dec 8, 2021 · 0 comments · Fixed by #12742
Closed

Add unit tests for SystemTimer.h #12729

kpschoedel opened this issue Dec 8, 2021 · 0 comments · Fixed by #12742
Assignees

Comments

@kpschoedel
Copy link
Contributor

Problem

Recent refactoring exposed new classes that aren't unit tested (and in fact contained at least one trivial bug #12723).

Proposed Solution

Add unit tests.

@kpschoedel kpschoedel self-assigned this Dec 8, 2021
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Dec 8, 2021
#### Problem

Only the higher-level `System::Layer` timer operations had tests;
the utility classes in `system/SystemTimer.h` had no unit tests,
and a recent refactor (project-chip#12628) introduced a bug that a proper unit
test would have caught.

Fixes project-chip#12729 Add unit tests for SystemTimer.h

#### Change overview

What's in this PR

- Add tests covering `TimerData`, `TimeList`, and `TimerPool`.
- Changed these helpers to take a `Timestamp` rather than a `Timeout`.
- Fixed `TimerList::Remove(Node*)` to allow an empty list or null
  argument (matching its description).

#### Testing

Quis custodiet ipsos custodes?
andy31415 pushed a commit that referenced this issue Dec 8, 2021
#### Problem

Only the higher-level `System::Layer` timer operations had tests;
the utility classes in `system/SystemTimer.h` had no unit tests,
and a recent refactor (#12628) introduced a bug that a proper unit
test would have caught.

Fixes #12729 Add unit tests for SystemTimer.h

#### Change overview

What's in this PR

- Add tests covering `TimerData`, `TimeList`, and `TimerPool`.
- Changed these helpers to take a `Timestamp` rather than a `Timeout`.
- Fixed `TimerList::Remove(Node*)` to allow an empty list or null
  argument (matching its description).

#### Testing

Quis custodiet ipsos custodes?
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 a pull request may close this issue.

1 participant