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

Replace linux-login by tty1-selected for firstrun.pm #619

Merged
merged 1 commit into from
Jan 30, 2020

Conversation

Zaoliang
Copy link
Contributor

@Zaoliang Zaoliang commented Nov 28, 2019

Copy link
Member

@SergioAtSUSE SergioAtSUSE left a comment

Choose a reason for hiding this comment

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

Why is it called tty6 if the screen that matches is tty1?

@foursixnine
Copy link
Member

foursixnine commented Nov 28, 2019

How does this behave in other cases?, where tty6-selected (or root console, according to the code in the text module)? because this one, seems to expects the OK messages to be shown there, always... so seems to be something very specific to JEOS, also.. tty6 != tty1!.

Perhaps rename the needle to include jeos in the name

@Zaoliang
Copy link
Contributor Author

just check this tty$Nr:

my $tty = get_root_console_tty;
assert_screen ["tty$tty-selected", 'reached-power-off'], 1000;

@Zaoliang Zaoliang changed the title Create new tty6-selected for firstrun.pm Create new tty1-selected for firstrun.pm Nov 28, 2019
Copy link
Member

@SergioAtSUSE SergioAtSUSE left a comment

Choose a reason for hiding this comment

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

But, I cannot see the verification run.

Copy link
Member

@foursixnine foursixnine left a comment

Choose a reason for hiding this comment

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

There seems to be changes missing on the test side of things according to this http://f40.suse.de/tests/5579/modules/firstrun/steps/1/src.

On another hand I wonder why jeos's console is not the right one (my $tty = get_root_console_tty; should return the right & expected one) from the beginning. @Zaoliang Could you figure that out as a follow up? /cc @SergioAtSUSE

@Zaoliang
Copy link
Contributor Author

http://f40.suse.de/tests/5579#step/firstrun/7
this is also updated in commit message.

@Zaoliang
Copy link
Contributor Author

I opened now for tty$Nr detection issue:
https://progress.opensuse.org/issues/60431

@SergioAtSUSE
Copy link
Member

SergioAtSUSE commented Nov 29, 2019

There seems to be changes missing on the test side of things according to this http://f40.suse.de/tests/5579/modules/firstrun/steps/1/src.

So, as already two person, including me, get confused by seeing only a PR with needles that should be a dependency of code change PR, @Zaoliang, please open also always the PR with the code changes, and put a link to the needle's PR in the description.

@SergioAtSUSE
Copy link
Member

SergioAtSUSE commented Nov 29, 2019

On another hand I wonder why jeos's console is not the right one (my $tty = get_root_console_tty; should return the right & expected one) from the beginning. @Zaoliang Could you figure that out as a follow up? /cc @SergioAtSUSE

If you mean, "why is get_root_console_tty returning two different consoles?":
https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/lib/utils.pm#L1055

If you mean, "why is tty1 being shown?", this is why I complained that we shouldn't expect root-console (in this case tty6) here, because we are at that point only checking if the system booted (also tty1 is expected there). The select_console statement happens later and then should be check for tty6 (or tty2, seeing get_root_console_tty() code)

@Zaoliang
Copy link
Contributor Author

anyway, this might be not a problem with detection of tty at this stage, but as reference which can be expected:
http://f40.suse.de/tests/5580#step/firstrun/10
so we had already tty1-selected for sles jeos.

@Zaoliang Zaoliang changed the title Create new tty1-selected for firstrun.pm Replace linux-login by tty1-selected for firstrun.pm Nov 29, 2019
@SergioAtSUSE SergioAtSUSE merged commit d9876d7 into os-autoinst:master Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants