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

Use serial_terminal to get a stable ghostscript #11786

Merged
merged 1 commit into from
Jan 28, 2021

Conversation

Zaoliang
Copy link
Contributor

@Zaoliang Zaoliang commented Jan 18, 2021

select_serial_terminal should be stable and resolves the sporadic
typing issue.
see https://progress.opensuse.org/issues/87674
verification:
http://10.160.64.152/tests/52#step/ghostscript/

Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

This won't scale. How do you envision our tests should look like if you sprinkle in type_string_slow in a specific case but not in general? I doubt this approach is sustainable.

If you see "performance problems" on specific workers then debug the specific workers. If you see stability problems in specific test modules then the test module should be designed accordingly. If you see a problem with the overall test flow or scenario then stability should be ensured on a higher level. For well-known infrastructure related limitations VNC_TYPING_LIMIT can be used.

@Zaoliang
Copy link
Contributor Author

VNC_TYPING_LIMIT is set already for 6, and I checked 4, it works though, but I don't want slow down whole tests.
DVD-installation has less performance, this is what experienced in past.

@mdoucha
Copy link
Contributor

mdoucha commented Jan 19, 2021

Running the setup commands in serial terminal and switching to VNC only to run gv and check needles would be much more reliable solution. Call $self->select_serial_terminal; to select the best available serial backend.

@okurz
Copy link
Member

okurz commented Jan 19, 2021

VNC_TYPING_LIMIT is set already for 6, and I checked 4, it works though, but I don't want slow down whole tests.
DVD-installation has less performance, this is what experienced in past.

I did not suggest to use VNC_TYPING_LIMIT in this specific case. Either find out what is causing trouble in the specific case and fix that or follow the advice from @mdoucha

tests/x11/ghostscript.pm Outdated Show resolved Hide resolved
Copy link
Contributor

@mdoucha mdoucha left a comment

Choose a reason for hiding this comment

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

Some comment cleanup and new verification run would be nice. Other than that, looks good to me.

@Zaoliang
Copy link
Contributor Author

this is not working yet:

http://10.160.64.152/tests/29#step/ghostscript/34

@mdoucha
Copy link
Contributor

mdoucha commented Jan 25, 2021

Oh, right. The test PDF file was written to /root/ but gv is looking for it in ~bernhard/. Replacing the typestring "exit\n"; on line 45 with $self->select_serial_terminal(0); will fix it.

@Zaoliang
Copy link
Contributor Author

$self->select_serial_terminal(0); failed :
see http://10.160.64.152/tests/35/file/autoinst-log.txt

�[0m[2021-01-25T14:06:53.271 CET] [debug] tests/x11/ghostscript.pm:45 called opensusebasetest::select_serial_terminal -> lib/opensusebasetest.pm:1243 called testapi::select_console
[2021-01-25T14:06:53.271 CET] [debug] <<< testapi::select_console(testapi_console="virtio-terminal")
console virtio-terminal does not exist at /usr/lib/os-autoinst/backend/driver.pm line 86.
�[33m[2021-01-25T14:06:53.319 CET] [info] ::: basetest::runtest: # Test died: Can't call method "select" on an undefined value at /usr/lib/os-autoinst/backend/baseclass.pm line 667.


@Zaoliang Zaoliang changed the title Use type_string_slow instead of assert_script_run for "wget" in ghostscript WIP Use type_string_slow instead of assert_script_run for "wget" in ghostscript Jan 25, 2021
@mdoucha
Copy link
Contributor

mdoucha commented Jan 25, 2021

Oh, great, somebody forgot to define a non-root virtio console... Never mind, you can just switch directory to '~'.$testapi::username instead. That'll also place the test PDF in the right directory.

@Zaoliang Zaoliang force-pushed the wget-command branch 2 times, most recently from 7118681 to fe529a8 Compare January 26, 2021 07:25
@Zaoliang Zaoliang changed the title WIP Use type_string_slow instead of assert_script_run for "wget" in ghostscript Use type_string_slow instead of assert_script_run for "wget" in ghostscript Jan 26, 2021
tests/x11/ghostscript.pm Outdated Show resolved Hide resolved
tests/x11/ghostscript.pm Outdated Show resolved Hide resolved
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

I still suggest to run tests itself after the preparation as the non-priviledged user and keep that part as before.

tests/x11/ghostscript.pm Outdated Show resolved Hide resolved
@mdoucha
Copy link
Contributor

mdoucha commented Jan 26, 2021

I still suggest to run tests itself after the preparation as the non-priviledged user and keep that part as before.

Yes, that would be nice, but it's currently not possible without workarounds like su bernhard because non-root virtio terminal is missing.

@okurz
Copy link
Member

okurz commented Jan 26, 2021

sure, I would be fine with su $username, can we do that? Maybe even in a separate bash process within

@Zaoliang Zaoliang changed the title Use type_string_slow instead of assert_script_run for "wget" in ghostscript WIP Use type_string_slow instead of assert_script_run for "wget" in ghostscript Jan 26, 2021
@Zaoliang Zaoliang force-pushed the wget-command branch 4 times, most recently from 0365e70 to 42d32ae Compare January 27, 2021 07:10
@Zaoliang Zaoliang changed the title WIP Use type_string_slow instead of assert_script_run for "wget" in ghostscript Use type_string_slow instead of assert_script_run for "wget" in ghostscript Jan 27, 2021
@mdoucha
Copy link
Contributor

mdoucha commented Jan 27, 2021

sure, I would be fine with su $username, can we do that? Maybe even in a separate bash process within

That pointless user switch is now messing up command prompt detection and slowing down the test to a crawl:
https://openqa.suse.de/tests/5350686#step/ghostscript/13

@Zaoliang
Copy link
Contributor Author

sure, I would be fine with su $username, can we do that? Maybe even in a separate bash process within

That pointless user switch is now messing up command prompt detection and slowing down the test to a crawl:
https://openqa.suse.de/tests/5350686#step/ghostscript/13

please provide suggestion here, if you are not happy with these changes from Oliver or yourself for user switch, I really don't understand your guys what you're expecting.

Copy link
Contributor

@mdoucha mdoucha left a comment

Choose a reason for hiding this comment

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

please provide suggestion here, if you are not happy with these changes from Oliver or yourself for user switch, I really don't understand your guys what you're expecting.

Sorry, my last comment was for Oliver. The test will work perfectly fine with just changing the directory and generating the PDF as root. Otherwise LGTM.

Comment on lines 50 to 51
type_string "su $username\n";
type_string "cd --\n";
Copy link
Member

Choose a reason for hiding this comment

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

Following @mdoucha's last suggestion:

Suggested change
type_string "su $username\n";
type_string "cd --\n";
my $working_directory = script_output("mktemp -d -t GHOSTSCRIPTXXX");
type_string "$working_directory\n";

Copy link
Member

Choose a reason for hiding this comment

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

Later before changing to x11, do:

script_run "chmod -R $username $working_directory"

Copy link
Member

Choose a reason for hiding this comment

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

once xterm is open, cd $working_directory

Copy link
Contributor

Choose a reason for hiding this comment

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

That's quite a lot of overkill. Simple assert_script_run('cd ~' . $testapi::username); will suffice. The rm -f at the end will remove files owned by root just fine. Once the missing virtio terminal gets added, we can simply swap the cd for select_user_serial_terminal().

@Zaoliang Zaoliang changed the title Use type_string_slow instead of assert_script_run for "wget" in ghostscript Use serial_terminal to get a stable ghostscript Jan 27, 2021
select_serial_terminal should be stable and resolves the sporadic
typing issue.
see https://progress.opensuse.org/issues/87674
verification:
http://10.160.64.152/tests/52#step/ghostscript
@Zaoliang
Copy link
Contributor Author

thanks for your input!

@Zaoliang Zaoliang merged commit 3871448 into os-autoinst:master Jan 28, 2021
@Zaoliang Zaoliang deleted the wget-command branch January 28, 2021 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants