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

menus.robot: added check if DUT reset succeded #448

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

menus.robot: added check if DUT reset succeded #448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 47 additions & 3 deletions lib/bios/menus.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Enter Boot Menu Tianocore

Get Boot Menu Construction
[Documentation] Keyword allows to get and return boot menu construction.
Sleep 1s
${menu}= Read From Terminal Until exit
# Lines to strip:
# TOP:
Expand Down Expand Up @@ -535,13 +536,36 @@ Remove Disk Password
END
Press Key N Times 1 ${SETUP_MENU_KEY}

Send Reboot Command
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we define new keyword and not use it?

Telnet.Write Bare \x1bR\x1br\x1bR

Tianocore Reset System
# EDK2 interprets Alt + Ctrl + Del on USB keyboards as reset combination.
# On serial console it is ESC R ESC r ESC R.
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
Copy link
Contributor

@macpijan macpijan Oct 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

${out}= Read From Terminal

# Clean up the string
Copy link
Contributor

@macpijan macpijan Oct 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@JanPrusinowski JanPrusinowski Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A long time has passed... I'm verifying if the issue still exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log2.zip
Issue still persists... I'll restore previous version and see if it will make any difference

${out}= Replace String ${out} \n ${EMPTY}
${out}= Replace String ${out} \t ${EMPTY}
${out}= Strip String ${out}

Set Test Variable ${CHECK} ${FALSE}
Copy link
Contributor

@macpijan macpijan Oct 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would appreciate some comments to help us understand the thought process more, and have easier time following the concepts in the code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

IF 'Continue' in '${out}'
IF 'Reset' in '${out}'
Set Test Variable ${CHECK} ${TRUE}
END
END

IF ${CHECK} == ${TRUE}
Go To Reset Option
BREAK
ELSE
Sleep 1s
Telnet.Write Bare ${ESC}
END
END
ELSE IF '${DUT_CONNECTION_METHOD}' == 'open-bmc'
FAIL OpenBMC not yet supported for interfacing with TianoCore
ELSE IF '${DUT_CONNECTION_METHOD}' == 'pikvm'
Expand All @@ -551,6 +575,26 @@ Tianocore Reset System
FAIL Unknown connection method for config: ${CONFIG}
END

Go To Reset Option
Set Test Variable ${MAX_TRIES} 20
FOR ${index} IN RANGE 0 ${MAX_TRIES}
${out}= Read From Terminal

# Clean up the string
${out}= Replace String ${out} \n ${EMPTY}
${out}= Replace String ${out} \t ${EMPTY}
${out}= Strip String ${out}

Sleep 1s
IF 'the language for the' in '${out}'
Telnet.Write Bare ${ENTER}
BREAK
ELSE
IF ${index} == ${MAX_TRIES} Fail
Telnet.Write Bare ${ARROW_UP}
END
END

Save Changes
[Documentation] Saves current UEFI settings
Press Key N Times 1 ${F10}
Expand Down
12 changes: 12 additions & 0 deletions self-tests/setup-and-boot-menus.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,15 @@ Get Menu Construction Stress Test
Run Keyword And Continue On Failure Should Not Contain ${line} disabled.
END
END

Tianocore Reset System Stress Test
FOR ${i} IN RANGE 1 50
Log To Console Run: ${i}
Power On
${sb_menu}= Enter Secure Boot Menu And Return Construction
Enable Secure Boot ${sb_menu}
Save Changes And Reset
# Tianocore Reset System
# Boot System Or From Connected Disk ubuntu
# Login To Linux
END
Loading