-
Notifications
You must be signed in to change notification settings - Fork 269
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
x11/network/NM_wpa2_enterprise: Do not click "Connect" in the second time #15866
Conversation
@@ -36,6 +38,7 @@ sub run { | |||
assert_and_click 'network_manager-wrong_card_selected'; | |||
assert_screen 'network_manager-network_connected'; | |||
} | |||
assert_and_click 'network_manager-popup-cancel'; |
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.
Hello @zhaocongjia
One minor comment here, can you please add some description here as well?
then everyone can know why we don't need to click connect button any more.
LGTM
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 saw there's no popup window for gnome43 but only a needle with tag "network_manager-popup-cancel" there which seems not real. Can we handle this more elegantly?
https://openqa.opensuse.org/tests/2861036#step/NM_wpa2_enterprise/34
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 current way handles both GNOME43 and older GNOME versions. Otherwise there will be check_screen
.
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.
Yeah, I know it works for all GNOME versions. But it looks weird to assert_and_click a "fake" needle.
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.
OK, I have a better idea. Wait for PR update.
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.
Hi, I have update code logic to remove the need of network_manager-popup-cancel
-- now we simply wait_still_screen
to wait for NetworkManager to auto reconnect.
https://openqa.opensuse.org/tests/2865773
https://openqa.opensuse.org/tests/2865779
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.
Great idea. LGTM.
47c54e9
to
fd237c1
Compare
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.
Sorry for failing to quote the code. I mean Line88 of the NM_wpa2_enterprise.pm
This could be simplified. Use type_password;
would be enough.
I know this is not introduced in this PR, it would be nice if this can be enhanced together with this PR.
fd237c1
to
a51a573
Compare
In the second time we connect to the wifi, there is no need to click "Connect" again -- we already typed password, so the system will auto-connect in the background.
Leap15.4: https://openqa.opensuse.org/tests/2861035#step/NM_wpa2_enterprise/34
TW: https://openqa.opensuse.org/tests/2861036