-
-
Notifications
You must be signed in to change notification settings - Fork 6
Reduce unnecessary reboots: part 1, AUD, MNE #1141
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
base: develop
Are you sure you want to change the base?
Conversation
d7d97af to
45b5dfd
Compare
|
Logs showing:
|
|
Splitting the suites into multiple files per OS was supposed to be a way to achieve less reboots during tests. Turns out, that this approach is essentially what was used in a PR to solve #1099 in #1130. The approach where we first gather all the data required for all tests (for a given OS) and then we only interpret the collected data in test cases is essentially what the solution for concurrent execution is. Splitting into multiple files, one per OS, allows to run data gathering on every OS independently. Just monitoring the power state and reducing reboots if not needed can work without this change. Just Logs after splitting the AUD test suite into files per OS. audio-subsystem_log.html |
07db682 to
b49faad
Compare
|
|
||
|
|
||
| *** Keywords *** | ||
| Init Power State Control |
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.
Would it make sense to have some tests for these new keywords on QEMU under self-tests to ensure they work properly?
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.
I guess the best way to test them would be to just do the same setup menu tests as now, just with the power state control ON and Power On Ex force_reboot=${TRUE} swapped to Power On Ex force_reboot=${FALSE}. Alternatively a separate test case could be added that just moves between setup/bootmenu/OS repeatedly a couple times.
| Power Cycle On | ||
| Boot State Control Notify State booting | ||
|
|
||
| Power On Ex |
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.
Ex stand for what exactly?
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.
Extended/Expanded, something to differentiate from Power On for which Power On Ex is a wrapper
| ELSE | ||
| Press Enter | ||
| Login To Linux Over Serial Console ${DEVICE_OS_USERNAME} ${DEVICE_OS_PASSWORD} | ||
| ${line}= Read From Terminal |
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.
should we explain why do we need to handle this case?
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.
I think the commit message should show more or less what's the intention: Login To Linux: Handle already logged in via serial. It's added so that if the OS is already booted and the user (or root) is already logged in, we can just continue instead of waiting for the login prompt (which will never appear)
|
Do you have some metrics - how long it took to run modified test suite - before and after the changes? |
Sure @macpijan , here's a direct comparison from logs:
That's 44% time reduction for just the automatic reboot reduction. Suprisingly after splitting into files-per-OS it gets even better at 51% time reduction. I'm nut quite sure what has changed, maybe it's just variation. |
This reverts commit e21ee821ea81d6552f52c029e2824c886519be17.
d12a3e1 to
3fdc06b
Compare
implementing changes for #748