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.

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

Fix the issue about imcomplete input on powershell #15010

Closed

Conversation

RoyCai7
Copy link
Contributor

@RoyCai7 RoyCai7 commented Jun 2, 2022

Imcomplete input on powershell
POO: https://progress.opensuse.org/issues/109091

@@ -32,6 +32,7 @@ sub run {
$self->run_in_powershell(cmd => 'Get-NetIPAddress -InterfaceAlias $a.name', tags => 'win-remote-desktop');
$self->run_in_powershell(cmd => 'New-NetIPAddress -InterfaceAlias $a.name -IPAddress 10.0.2.18 -PrefixLength 24 -DefaultGateway 10.0.2.2 -Confirm:$false', tags => 'win-remote-desktop');
approve_network_popup;
sleep 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

wait_still_screen 3;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sleep 3 is passed.

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 try to use wait_still_screen

@RoyCai7 RoyCai7 force-pushed the windows_network_setup_109091 branch from 2669594 to 7e8f4b3 Compare June 2, 2022 10:41
@RoyCai7 RoyCai7 marked this pull request as ready for review June 6, 2022 01:02
@@ -32,6 +32,7 @@ sub run {
$self->run_in_powershell(cmd => 'Get-NetIPAddress -InterfaceAlias $a.name', tags => 'win-remote-desktop');
$self->run_in_powershell(cmd => 'New-NetIPAddress -InterfaceAlias $a.name -IPAddress 10.0.2.18 -PrefixLength 24 -DefaultGateway 10.0.2.2 -Confirm:$false', tags => 'win-remote-desktop');
approve_network_popup;
wait_still_screen 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM, could you please add some comments here why you add 'sleep‘ here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add some comments and tags.

@RoyCai7 RoyCai7 force-pushed the windows_network_setup_109091 branch from 7e8f4b3 to 5c9ed02 Compare June 6, 2022 02:52
Comment on lines 35 to 36
# Imcomplete input on powershell because executing command is too fast
# Poo#109091
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest change:
We may miss some characters due to type command too fast after network popup, so wait several seconds as a workaround,
See poo#109091

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

@RoyCai7 RoyCai7 force-pushed the windows_network_setup_109091 branch from 5c9ed02 to c5a7d52 Compare June 6, 2022 03:12
Copy link
Contributor

@rfan1 rfan1 left a comment

Choose a reason for hiding this comment

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

LGTM

@rfan1
Copy link
Contributor

rfan1 commented Jun 6, 2022

@GraceWang571
Could you please help review this PR?

Copy link
Contributor

@GraceWang571 GraceWang571 left a comment

Choose a reason for hiding this comment

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

I see many test cases invoke run_in_powershell, I'd like to say add wait_still_screen in run_in_powershell would be a better choice.
You know there might be similar failure when invoking run_in_powershell without wait_still_screen before OR in it.

@jouyingbin
Copy link
Contributor

@Wangqianzou there is typo in your subject and git commit message.

typo: imcomplete - > incomplete

@rfan1
Copy link
Contributor

rfan1 commented Jun 6, 2022

I see many test cases invoke run_in_powershell, I'd like to say add wait_still_screen in run_in_powershell would be a better choice. You know there might be similar failure when invoking run_in_powershell without wait_still_screen before OR in it.

Actually, I talked with Roy last week, and I had the same suggestion, however, for this single case, the issue was not related to powershell script, since the issue seems happen after:

sub approve_network_popup {
# if there is the networks popup that ask's if it's ok to be discoverable, approve it
wait_still_screen stilltime => 5, timeout => 10;
assert_screen([qw(networks-popup-be-discoverable no-network-discover-popup)], 60);
if (match_has_tag 'networks-popup-be-discoverable') {
assert_and_click 'network-discover-yes';
wait_screen_change(sub { send_key 'ret' }, 10);
}
}

@rfan1
Copy link
Contributor

rfan1 commented Jun 6, 2022

I see many test cases invoke run_in_powershell, I'd like to say add wait_still_screen in run_in_powershell would be a better choice. You know there might be similar failure when invoking run_in_powershell without wait_still_screen before OR in it.

Actually, I talked with Roy last week, and I had the same suggestion, however, for this single case, the issue was not related to powershell script, since the issue seems happen after:

sub approve_network_popup { # if there is the networks popup that ask's if it's ok to be discoverable, approve it wait_still_screen stilltime => 5, timeout => 10; assert_screen([qw(networks-popup-be-discoverable no-network-discover-popup)], 60); if (match_has_tag 'networks-popup-be-discoverable') { assert_and_click 'network-discover-yes'; wait_screen_change(sub { send_key 'ret' }, 10); } }

BTW, add wait_still_screen to un_in_powershell may consume more time :)

@GraceWang571
Copy link
Contributor

Actually, I talked with Roy last week, and I had the same suggestion, however, for this single case, the issue was not related to powershell script, since the issue seems happen after:

sub approve_network_popup { # if there is the networks popup that ask's if it's ok to be discoverable, approve it wait_still_screen stilltime => 5, timeout => 10; assert_screen([qw(networks-popup-be-discoverable no-network-discover-popup)], 60); if (match_has_tag 'networks-popup-be-discoverable') { assert_and_click 'network-discover-yes'; wait_screen_change(sub { send_key 'ret' }, 10); } }

I am afraid I can't agree with you. As you can see the approve_network_popup; was invoked twice in windows_network_setup.pm. And Roy only made changes for one of them and then the test case passed.

As you might know, poo#109091 is a performance issue and can't be reproduced 100%. Our goal is to make the test code robust to avoid the failures caused by the possible performance.

@rfan1
Copy link
Contributor

rfan1 commented Jun 6, 2022

@Wangqianzou
As offline discussion with @GraceWang571,

I would suggest you add the workaround into function approve_network_popup. since We use this function more than once!

And if you have seen any performance issue with run_in_powershell, you may add the similiar workaround as well to this function. [as far as I know, we rarely hit that issue before :)]

@rfan1
Copy link
Contributor

rfan1 commented Jun 7, 2022

#15017

@Wangqianzou You can close this PR once you back from vacation.

@RoyCai7 RoyCai7 closed this Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants