-
-
Notifications
You must be signed in to change notification settings - Fork 39.8k
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
[Test] Reset timer for every unit test and provide timestamps for log messages #17028
[Test] Reset timer for every unit test and provide timestamps for log messages #17028
Conversation
3f65739
to
1b25ffc
Compare
CC: @filterpaper @precondition @joukewitteveen as you are involved with unit tests recently and might this feature useful? |
I'd add an explanation of what the numbers after [TRACE] mean in the docs page on unit tests. It's not obvious that it is a timestamp. |
1b25ffc
to
475a2c9
Compare
fabf77e
to
66a73fc
Compare
@precondition I did go a bit further and included a header for the output that explains each column. For better introspection I prototyped a mechanism that resolves the usb report keys and mods to readable constants, furthermore the QMK keycode for each button is now stored in the keycode on construction (although with a bit ugly C stringify magic). I updated the log output in the description of this PR accordingly. |
5b9f7b6
to
7e4d888
Compare
I did not test the code in this PR and the changes in #16394 would be affected by the changes in this PR: the In case of failing tests related to tap dance, I can see how it can be pleasant to restart the timer for each test. That would keep the numbers down a bit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__attribute__((weak))
✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tap dance tests added with #16394 should also be updated to use the KEYMAP_KEY
macro.
8d84a65
to
fb21c93
Compare
44640ac
to
6b8d1b6
Compare
Note that these updates of the tap dance tests are still missing from this PR. |
6b8d1b6
to
07bccf0
Compare
Thank you for your contribution! |
On hold until DD keycodes have been merged |
But #18643 had been merged… or do you mean something else? |
You are right, I missed that. I'll generate the keycode lookup table from the keycode specifications. |
07bccf0
to
878fc32
Compare
6d1e585
to
c44c3a3
Compare
Converted the keycode lookup table to be generated from keycode definitions and extended the unit-tests. This is now ready for review. |
c44c3a3
to
838ae83
Compare
838ae83
to
1ec9505
Compare
Description
Previously the unit-tests where run with a continuously running timer, which can trigger timing bugs in unrelated unit tests. To remedy this we now reset the timer for each unit test. All events in log messages now also contain a timestamp and key events show how long they have been held/released. This is meant as a convenience feature for debugging and developing unit-tests.
e.g. the quite complex
ANewTapWithinTappingTermIsBuggy
test on failure before:and with these changes:
Types of Changes
Issues Fixed or Closed by This PR
keyevent_t
for 1ms timing resolution #15847 which will be WIP for longer time.Checklist