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.

Sign up for GitHub

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

Jump to bottom

Add unit tests for SystemTimer.h #12742

Merged
merged 1 commit into from
Dec 8, 2021
Merged

Add unit tests for SystemTimer.h #12742

merged 1 commit into from
Dec 8, 2021

Conversation

kpschoedel
Copy link
Contributor

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?

@kpschoedel
Add unit tests for SystemTimer.h
9b350b8 
#### 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?
@boring-cyborg boring-cyborg bot added the system label Dec 8, 2021
@github-actions
Copy link

github-actions bot commented Dec 8, 2021 •
edited
Loading

PR #12742: Size comparison from 2534981 to 9b350b8

Increases (14 builds for esp32, nrfconnect, qpg, telink)
platform target config section 2534981 9b350b8 change % change
esp32 all-clusters-app c3devkit (read only) 856546 856556 10 0.0
.flash.text 856546 856556 10 0.0
m5stack (read only) 959467 959479 12 0.0
.flash.text 954083 954095 12 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 897531 897547 16 0.0
text 605592 605612 20 0.0
nrf52840dk_nrf52840+rpc (read/write) 860555 860571 16 0.0
text 580320 580340 20 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 823478 823494 16 0.0
text 535972 535992 20 0.0
lock-app nrf52840dk_nrf52840 (read/write) 870003 870019 16 0.0
text 584852 584868 16 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 796186 796218 32 0.0
text 515324 515344 20 0.0
pump-app nrf52840dk_nrf52840 (read/write) 874851 874867 16 0.0
text 588368 588388 20 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 868075 868091 16 0.0
text 583588 583604 16 0.0
shell nrf52840dk_nrf52840 (read/write) 781611 781627 16 0.0
text 523264 523280 16 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 696686 696702 16 0.0
text 443904 443924 20 0.0
qpg lighting-app qpg6100+debug (read only) 512436 512460 24 0.0
.text 507116 507140 24 0.0
lock-app qpg6100+debug (read only) 486644 486668 24 0.0
.text 481324 481348 24 0.0
telink lighting-app tlsr9518adk80d (read/write) 801002 801018 16 0.0
text 558038 558054 16 0.0
Decreases (11 builds for efr32, k32w, linux, p6)
platform target config section 2534981 9b350b8 change % change
efr32 lighting-app BRD4161A (read only) 795788 795772 -16 -0.0
.text 795780 795764 -16 -0.0
BRD4161A+rpc (read only) 783400 783384 -16 -0.0
.text 783392 783376 -16 -0.0
window-app BRD4161A (read only) 772740 772724 -16 -0.0
.text 772732 772716 -16 -0.0
k32w lighting-app k32w061+se05x+release (read/write) 732624 732608 -16 -0.0
.text 643724 643708 -16 -0.0
lock-app k32w061+debug (read/write) 623224 623192 -32 -0.0
.text 543692 543660 -32 -0.0
shell k32w061+debug (read/write) 686316 686300 -16 -0.0
.text 597112 597096 -16 -0.0
linux chip-tool-ipv6only arm64 (read only) 6727444 6727428 -16 -0.0
.text 5728388 5728372 -16 -0.0
thermostat-no-ble arm64 (read only) 1908972 1908932 -40 -0.0
.rodata 123964 123956 -8 -0.0
.text 1579968 1579936 -32 -0.0
p6 all-clusters-app default (read/write) 2351232 2351216 -16 -0.0
.text 1309496 1309480 -16 -0.0
light-app default (read/write) 2282296 2282280 -16 -0.0
.text 1240560 1240544 -16 -0.0
lock-app default (read/write) 2258400 2258384 -16 -0.0
.text 1216664 1216648 -16 -0.0
Full report (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 2534981 9b350b8 change % change
efr32 lighting-app BRD4161A (read only) 795788 795772 -16 -0.0
(read/write) 122996 122996 0 0.0
.bss 121176 121176 0 0.0
.data 1820 1820 0 0.0
.text 795780 795764 -16 -0.0
BRD4161A+rpc (read only) 783400 783384 -16 -0.0
(read/write) 139680 139680 0 0.0
.bss 137752 137752 0 0.0
.data 1928 1928 0 0.0
.text 783392 783376 -16 -0.0
window-app BRD4161A (read only) 772740 772724 -16 -0.0
(read/write) 121136 121136 0 0.0
.bss 119352 119352 0 0.0
.data 1784 1784 0 0.0
.text 772732 772716 -16 -0.0
esp32 all-clusters-app c3devkit (read only) 856546 856556 10 0.0
(read/write) 1306058 1306058 0 0.0
.dram0.bss 67648 67648 0 0.0
.dram0.data 14124 14124 0 0.0
.flash.rodata 171256 171256 0 0.0
.flash.text 856546 856556 10 0.0
.iram0.text 62076 62076 0 0.0
m5stack (read only) 959467 959479 12 0.0
(read/write) 450284 450284 0 0.0
.dram0.bss 75000 75000 0 0.0
.dram0.data 34048 34048 0 0.0
.flash.rodata 209448 209448 0 0.0
.flash.text 954083 954095 12 0.0
.iram0.text 123451 123451 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 732624 732608 -16 -0.0
.bss 81248 81248 0 0.0
.data 1852 1852 0 0.0
.text 643724 643708 -16 -0.0
lock-app k32w061+debug (read/write) 623224 623192 -32 -0.0
.bss 71912 71912 0 0.0
.data 1820 1820 0 0.0
.text 543692 543660 -32 -0.0
shell k32w061+debug (read/write) 686316 686300 -16 -0.0
.bss 81612 81612 0 0.0
.data 1792 1792 0 0.0
.text 597112 597096 -16 -0.0
linux chip-tool-ipv6only arm64 (read only) 6727444 6727428 -16 -0.0
(read/write) 310625 310625 0 0.0
.bss 51665 51665 0 0.0
.data 1048 1048 0 0.0
.data.rel.ro 206312 206312 0 0.0
.dynamic 560 560 0 0.0
.got 47960 47960 0 0.0
.init 24 24 0 0.0
.init_array 160 160 0 0.0
.rodata 341588 341588 0 0.0
.text 5728388 5728372 -16 -0.0
thermostat-no-ble arm64 (read only) 1908972 1908932 -40 -0.0
(read/write) 135921 135921 0 0.0
.bss 59457 59457 0 0.0
.data 776 776 0 0.0
.data.rel.ro 69376 69376 0 0.0
.dynamic 560 560 0 0.0
.got 3456 3456 0 0.0
.init 24 24 0 0.0
.init_array 256 256 0 0.0
.rodata 123964 123956 -8 -0.0
.text 1579968 1579936 -32 -0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2317032 2317032 0 0.0
.bss 186900 186900 0 0.0
.data 5232 5232 0 0.0
.heap 844312 844312 0 0.0
.text 1279608 1279608 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2299488 2299488 0 0.0
.bss 175712 175712 0 0.0
.data 5488 5488 0 0.0
.heap 855248 855248 0 0.0
.text 1262088 1262088 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2272464 2272464 0 0.0
.bss 174752 174752 0 0.0
.data 5488 5488 0 0.0
.heap 856208 856208 0 0.0
.text 1235064 1235064 0 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1140008 1140008 0 0.0
.bss 11756 11756 0 0.0
.data 4376 4376 0 0.0
.heap 1020312 1020312 0 0.0
.text 103392 103392 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2047808 2047808 0 0.0
.bss 156564 156564 0 0.0
.data 4864 4864 0 0.0
.heap 875016 875016 0 0.0
.text 1010408 1010408 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 897531 897547 16 0.0
bss 116184 116184 0 0.0
rodata 100196 100196 0 0.0
text 605592 605612 20 0.0
nrf52840dk_nrf52840+rpc (read/write) 860555 860571 16 0.0
bss 112532 112532 0 0.0
rodata 91492 91492 0 0.0
text 580320 580340 20 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 823478 823494 16 0.0
bss 117560 117560 0 0.0
rodata 95456 95456 0 0.0
text 535972 535992 20 0.0
lock-app nrf52840dk_nrf52840 (read/write) 870003 870019 16 0.0
bss 113448 113448 0 0.0
rodata 96316 96316 0 0.0
text 584852 584868 16 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 796186 796218 32 0.0
bss 114856 114856 0 0.0
rodata 91604 91604 0 0.0
text 515324 515344 20 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 497463 497463 0 0.0
bss 51820 51820 0 0.0
rodata 45852 45852 0 0.0
text 339492 339492 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 874851 874867 16 0.0
bss 113360 113360 0 0.0
rodata 97668 97668 0 0.0
text 588368 588388 20 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 868075 868091 16 0.0
bss 113236 113236 0 0.0
rodata 95804 95804 0 0.0
text 583588 583604 16 0.0
shell nrf52840dk_nrf52840 (read/write) 781611 781627 16 0.0
bss 109552 109552 0 0.0
rodata 74280 74280 0 0.0
text 523264 523280 16 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 696686 696702 16 0.0
bss 110532 110532 0 0.0
rodata 68920 68920 0 0.0
text 443904 443924 20 0.0
p6 all-clusters-app default (read/write) 2351232 2351216 -16 -0.0
.bss 113132 113132 0 0.0
.data 2512 2512 0 0.0
.heap 917696 917696 0 0.0
.text 1309496 1309480 -16 -0.0
light-app default (read/write) 2282296 2282280 -16 -0.0
.bss 100904 100904 0 0.0
.data 2328 2328 0 0.0
.heap 930112 930112 0 0.0
.text 1240560 1240544 -16 -0.0
lock-app default (read/write) 2258400 2258384 -16 -0.0
.bss 99784 99784 0 0.0
.data 2288 2288 0 0.0
.heap 931272 931272 0 0.0
.text 1216664 1216648 -16 -0.0
qpg lighting-app qpg6100+debug (read only) 512436 512460 24 0.0
(read/write) 122332 122332 0 0.0
.bss 82624 82624 0 0.0
.data 956 956 0 0.0
.text 507116 507140 24 0.0
lock-app qpg6100+debug (read only) 486644 486668 24 0.0
(read/write) 122336 122336 0 0.0
.bss 81760 81760 0 0.0
.data 912 912 0 0.0
.text 481324 481348 24 0.0
persistent-storage-app qpg6100+debug (read only) 108104 108104 0 0.0
(read/write) 122336 122336 0 0.0
.bss 36152 36152 0 0.0
.data 288 288 0 0.0
.text 102784 102784 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 801002 801018 16 0.0
bss 82764 82764 0 0.0
noinit 37160 37160 0 0.0
text 558038 558054 16 0.0

@andy31415 andy31415 merged commit 8622381 into project-chip:master Dec 8, 2021
@kpschoedel kpschoedel deleted the system-timer-tests branch December 8, 2021 21:11
@wqx6 wqx6 mentioned this pull request Dec 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yunhanw-google yunhanw-google yunhanw-google approved these changes

@anush-apple anush-apple Awaiting requested review from anush-apple

@austinh0 austinh0 Awaiting requested review from austinh0

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

@carol-apple carol-apple Awaiting requested review from carol-apple

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chulspro chulspro Awaiting requested review from chulspro

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@hawk248 hawk248 Awaiting requested review from hawk248

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@LuDuda LuDuda Awaiting requested review from LuDuda

@lzgrablic02 lzgrablic02 Awaiting requested review from lzgrablic02

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@pan-apple pan-apple Awaiting requested review from pan-apple

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@tecimovic tecimovic Awaiting requested review from tecimovic

@vijs vijs Awaiting requested review from vijs

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@woody-apple woody-apple Awaiting requested review from woody-apple

Assignees
No one assigned
Labels
fast track review - pending system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add unit tests for SystemTimer.h
3 participants
@kpschoedel @yunhanw-google @andy31415