menus.robot: added check if DUT reset succeded #448
Conversation
JanPrusinowski
force-pushed
the
356-tianocore-reset-s-1
branch
from
85a13c1
to
bf78b2e
Compare
setup-and-boot-menus.robot: Added stress test for Tianocore Reset System Signed-off-by: Jan Prusinowski <jan.prusinowski@3mdeb.com>
JanPrusinowski
force-pushed
the
356-tianocore-reset-s-1
branch
from
bf78b2e
to
11eadb7
Compare
| @@ -535,13 +536,36 @@ Remove Disk Password | |||
| END | |||
| Press Key N Times 1 ${SETUP_MENU_KEY} | |||
|
|
|||
| Send Reboot Command | |||
There was a problem hiding this comment.
Why do we define new keyword and not use it?
| IF '${DUT_CONNECTION_METHOD}' == 'SSH' | ||
| FAIL SSH not supported for interfacing with TianoCore | ||
| ELSE IF '${DUT_CONNECTION_METHOD}' == 'Telnet' | ||
| Telnet.Write Bare \x1bR\x1br\x1bR | ||
| FOR ${i} IN RANGE 0 5 |
There was a problem hiding this comment.
I do not like that we change it for Telnet only. It means now we will have two different methods of saving changes. One - more manual - for Telnet, the other one - for PiKVM. This sounds like asking for even more problems.
| FOR ${i} IN RANGE 0 5 | ||
| ${out}= Read From Terminal | ||
|
|
||
| # Clean up the string |
There was a problem hiding this comment.
Do we need to implement it once again?
In the history of this function, we can see the "manual" method has been replaced with the CTRL+ALT+DELETE here: ee72aad
If we can prove that:
- the "new" method was less reliable
- the "old" method was more reliable
We can simply bring back the old method by reverting this commit.
On top of this, we can add some improvements, if necessary, but I would start with simply bringing the old way.
There was a problem hiding this comment.
A long time has passed... I'm verifying if the issue still exists.
There was a problem hiding this comment.
log2.zip
Issue still persists... I'll restore previous version and see if it will make any difference
| ${out}= Replace String ${out} \t ${EMPTY} | ||
| ${out}= Strip String ${out} | ||
|
|
||
| Set Test Variable ${CHECK} ${FALSE} |
There was a problem hiding this comment.
I would appreciate some comments to help us understand the thought process more, and have easier time following the concepts in the code
There was a problem hiding this comment.
I have tested 3 options for this keyword.
Current one - Which is the least "stable" There is an issue where sending CTRL+ALT+DEL in dasharo just does not reset the platform. I have tried sending this shortcut even 2 or 3 times - and the reboot was not always guaranteed. Actually about 1 in 5 times this would fail.
The one I wrote and submitted in this PR. I know the solution isnt perfect as it is - but it is only a proof of concept - and this seemed to work much better. It still had its hickups and hung from time to time. I wrote some tests which rebooted the DUT 50 times using this method and sometimes the test has passed. Sometimes it hung on 40+th try... But more testing should be done and this method should be refined.
3rd method was the 'old' implementation of the kwd (which stored how many times you should press enter and up arrow to get to the default cursor position in dasharo) <- i have run this kwd only once (50 times) and it looked pretty stable - i had to stop the test after some 40th try but as it actually works similarly to the method I tried to write IMO reverting the change to this kwd would greatly improve tests stability.
No description provided.