-
Notifications
You must be signed in to change notification settings - Fork 64
add a check if wakealarm is supported (bugfix) #1921
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1921 +/- ##
==========================================
- Coverage 50.47% 50.22% -0.25%
==========================================
Files 382 382
Lines 41055 40809 -246
Branches 6895 6847 -48
==========================================
- Hits 20722 20496 -226
+ Misses 19587 19579 -8
+ Partials 746 734 -12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This avoids a failing test and skips it instead if the rtc does not support an automatic wakeup.
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.
Thanks for your contribution!
I left a comment inline.
Can you describe the use case you have and the issue you're encountering? This job is very sensitive because it's used in almost every test plan we maintain, so modifying it is always a bit risky.
This is a easier to read syntax which essentially does the same thing. Co-authored-by: Pierre Equoy <[email protected]>
Currently, I am running Checkbox on hardware that does not support the wakealarm functionality. My thought is that if the interface or functionality is not supported, failing this test - based solely on rtc.state == supported and mem.sleep == supported - is incorrect. I would assume that checking for wakealarm functionality should be a requirement for this since it depends on this. After giving it some more thought you could also interpret suspendability as a basic requirement. Then failing this would be the correct choice. This thought only occured after creating this pull request :) |
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in a week. |
This PR was closed because of inactivity. Feel free to re-open this once you want to work on it again! |
Description
This avoids a failing test and skips it instead if the rtc does not support an automatic wakeup.
The
suspend_advanced_auto
job is failing even though it cannot be run when the wakealarm is not supported.Resolved issues
N/A
Documentation
N/A
Tests
N/A