Skip to content

(#2179309) test: make TEST-35-LOGIN stable again#374

Merged
systemd-rhel-bot merged 1 commit intoredhat-plumbers:mainfrom
mrc0mmand:TEST-35-tweaks
Mar 30, 2023
Merged

(#2179309) test: make TEST-35-LOGIN stable again#374
systemd-rhel-bot merged 1 commit intoredhat-plumbers:mainfrom
mrc0mmand:TEST-35-tweaks

Conversation

@mrc0mmand
Copy link
Member

Let's add a couple of synchronization points and reduce the amount of sleeps to make the test, hopefully, stable again.

Follow-up to 638c241.
Related: #2179309
rhel-only


Let's see if the CI agrees, locally the test appeared quite stable once again.

@mrc0mmand mrc0mmand requested a review from dtardon March 29, 2023 20:42
@mergify mergify bot added the pr/needs-ci Formerly needs-ci label Mar 29, 2023
@systemd-rhel-bot systemd-rhel-bot added the pr/needs-review Formerly needs-review label Mar 29, 2023
@systemd-rhel-bot systemd-rhel-bot changed the title test: make TEST-35-LOGIN stable again (#2179309) test: make TEST-35-LOGIN stable again Mar 29, 2023
@systemd-rhel-bot systemd-rhel-bot added the tracker/unapproved Formerly needs-acks label Mar 29, 2023
@mrc0mmand mrc0mmand marked this pull request as draft March 29, 2023 21:33
@mrc0mmand
Copy link
Member Author

Welp, CI definitely disagrees. The search goes on then...

dtardon
dtardon previously approved these changes Mar 30, 2023
Copy link
Member

@dtardon dtardon left a comment

Choose a reason for hiding this comment

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

Anything that makes the test more reliable is good.

LGTM

@systemd-rhel-bot systemd-rhel-bot removed the pr/needs-review Formerly needs-review label Mar 30, 2023
@dtardon
Copy link
Member

dtardon commented Mar 30, 2023

Btw, maybe you could take inspiration from systemd/systemd#27038 and use a fifo for synchronization with the expect process instead of a busy loop?

The expect stuff was anything but expected, so let's just backport
the upstream test case and tweak it a bit to account for the missing
parts in our downstream testing infrastructure.

Follow-up to 638c241.
Related: #2179309
rhel-only
@mrc0mmand
Copy link
Member Author

Okay, so I gave up on the expect stuff, since it did everything but what I'd expect, and decided to just backport the latest version of the test from the upstream. After tweaking it a little, to account for missing stuff in downstream, it seems to be pretty stable (at least it keeps passing locally when I run it in a loop). Let's see what the CI is going to say.

@mrc0mmand mrc0mmand marked this pull request as ready for review March 30, 2023 10:17
@msekletar
Copy link
Member

LGTM and CI is finally green 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants