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 screen share test in SLE 15-SP4 #14480

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

DeepthiYV
Copy link
Contributor

@DeepthiYV DeepthiYV commented Mar 14, 2022

vino is considered out-of-date, and now gnome-remote-desktop progressively provides the screen sharing.
So I have renamed the test module vino_screensharing_available to screensharing_available to verify the availability of screen share across all SLE Versions. If the screen sharing is not available then test does the installation of package gnome-remote-desktop in SLE 15 SP4 and in lower version vino package.

https://gitlab.gnome.org/GNOME/gnome-control-center/-/merge_requests/767

@github-actions
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

  • Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files


# Install the vino package which is probably the case of missing screen sharing option
ensure_installed 'vino';
if (is_sle("=15-sp4")) {
Copy link
Contributor

@dzedro dzedro Mar 16, 2022

Choose a reason for hiding this comment

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

I would use 15-SP4+, I don't think on next SLE version it will go back to vino.
Applies for all is_sle

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I have updated the code to 15-SP4+.

@punkioudi
Copy link
Contributor

Hi Dee, could you explain a bit more the purpose of the PR?
In the description you write Replaced vino to gnome-remote-desktop, but the module is called vino_screensharing_available

@DeepthiYV
Copy link
Contributor Author

Hi Dee, could you explain a bit more the purpose of the PR? In the description you write Replaced vino to gnome-remote-desktop, but the module is called vino_screensharing_available

Hi Anna, I updated the description above and let me know is this ok :)

if (is_sle("=15-sp4")) {
record_info 'gnome-remote-desktop', 'After the installation the screen sharing is not available - gnome-remote-dekstop is missing and we need to install it now.';
send_key 'ctrl-q';
# Install the vino package which is probably the case of missing screen sharing option
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Install the vino package which is probably the case of missing screen sharing option
# Install the gnome-desktop package which is probably the case of missing screen sharing option

@punkioudi
Copy link
Contributor

Hi Dee, could you explain a bit more the purpose of the PR? In the description you write Replaced vino to gnome-remote-desktop, but the module is called vino_screensharing_available

Hi Anna, I updated the description above and let me know is this ok :)

Still the title of the module seems misleading to me :P

ensure_installed 'gnome-remote-desktop';
}
else {
record_info 'vino missing', 'After the installation the screen sharing is not available - vino is missing and we need to install it now.';
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace also vino here with gnome-desktop?

# Install the vino package which is probably the case of missing screen sharing option
ensure_installed 'vino';
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment on why vino is still tested, i.e (Or a link where this has been mentioned or discusse)

@@ -64,7 +69,7 @@ sub run {
record_soft_failure 'boo#1137569 - screen sharing not yet supported on wayland';
} else {
assert_screen 'with_screensharing';
record_info 'vino present', 'Vino and the screen sharing are present';
is_sle("=15-sp4") ? record_info('gnome-remote-desktop present and the screen sharing are present') : record_info('vino present', 'Vino and the screen sharing are present');
Copy link
Member

Choose a reason for hiding this comment

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

see above comment from @dzedro

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 the comment. Thanks :)

@DeepthiYV
Copy link
Contributor Author

Hi Dee, could you explain a bit more the purpose of the PR? In the description you write Replaced vino to gnome-remote-desktop, but the module is called vino_screensharing_available

Hi Anna, I updated the description above and let me know is this ok :)

Sure Anna, As we discussed offline, I have renamed module to generic name and verified the test. Please let me know your feedback. Thanks :)

@@ -1309,6 +1309,7 @@ sub load_x11tests {
loadtest "x11/eog";
loadtest(is_sle('<15') ? "x11/rhythmbox" : "x11/gnome_music");
loadtest "x11/wireshark";
loadtest "x11/remote_desktop/screensharing_available" if is_sle("=15-sp4");
Copy link
Contributor

@dzedro dzedro Mar 21, 2022

Choose a reason for hiding this comment

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

Why ? If difference between 15sp4+ and older versions is handled in the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Jozef, I scheduled screensharing_available test in we-module as gnome-remote-desktop is present SUSE Linux Enterprise Workstation Extension 15 SP4. I am still not sure how we are scheduling test. Let me know is this OK or do I need to do any changes?

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 discussed with Santiago and decided to move screensharing_available test to yaml schedule. I created a new ticket https://progress.opensuse.org/issues/109373.

ensure_installed 'vino';

if (is_sle("15-sp4+")) {
record_info 'gnome-remote-desktop', 'After the installation the screen sharing is not available - gnome-remote-dekstop is missing and we need to install it now.';
Copy link
Member

Choose a reason for hiding this comment

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

is there a bug for this? I think this should be a soft failure (same below)... I.E we should know what's the expectation and expected result after the installation?. Maybe a separate ticket to figure that out and update later (and actually use the screen sharing)... to keep things moving

Copy link
Contributor Author

@DeepthiYV DeepthiYV Mar 30, 2022

Choose a reason for hiding this comment

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

There is no bug. In SLE-15 SP4, screen sharing is available by default and we do not required to install any package. I removed the code for installing the package if screen sharing is not available.

@DeepthiYV DeepthiYV force-pushed the vino_screenshare_gnome branch 3 times, most recently from 24ae437 to 3d9e33a Compare March 30, 2022 14:49
@@ -1335,6 +1335,7 @@ sub load_x11tests {
loadtest(is_sle('<15') ? "x11/rhythmbox" : "x11/gnome_music");
loadtest "x11/wireshark";
loadtest "x11/ImageMagick";
loadtest "x11/remote_desktop/screensharing_available" if is_sle("15-sp4+");
Copy link
Member

Choose a reason for hiding this comment

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

Once you create the ticket for the extraction, please add the link to this PR

Copy link
Member

Choose a reason for hiding this comment

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

@foursixnine
Copy link
Member

Jozef is still FTO

Use gnome-remote-desktop to share the screen in SLE 15 SP4. Renamed the test vino_screensharing_available
to generic screensharing_available to test the screen share availability across all SLE flavors
@foursixnine foursixnine merged commit fc406e3 into os-autoinst:master Apr 12, 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