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

apps: Fix crash with nonexisting /etc/os-release #19416

Merged
merged 2 commits into from
Sep 29, 2023

Conversation

martinpitt
Copy link
Member

Broken out from PR #19281, as this is an embarassing crash/regression.

Explicitly check that the metapackage gets installed. This makes it
easier to tell apart mere "too short timeout" failures from persistent
ones.
@martinpitt martinpitt added the release-blocker Targetted for next release label Sep 29, 2023
m.write("/etc/os-release", 'ID="derivative"\nID_LIKE="spicy testy"\nVERSION_ID="1"\n')
b.click("#refresh")
with b.wait_timeout(30):
b.wait_visible(".app-list #app-1")
m.execute("until test -e /usr/share/app-info/xmls/test.xml; do sleep 1; done")
Copy link
Member

Choose a reason for hiding this comment

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

What about using b.wait for the loop? Then we can control the timeout in the usual way and don't need to wait for the global timeout when this fails.

Copy link
Member Author

Choose a reason for hiding this comment

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

m.execute() has a default timeout of 2 minutes; it can be changed with timeout=, but the default seemed quite appropriate to me. We often use this pattern in our tests, see git grep -E 'execute.*(while|until)'

Copy link
Member Author

Choose a reason for hiding this comment

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

and wait() (not b.wait() !) is more expensive, as it creates new SSH connections for each iteration.

Copy link
Member

Choose a reason for hiding this comment

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

m.execute() has a default timeout of 2 minutes

Ah, I didn't think of that!

@mvollmer mvollmer merged commit 93e8353 into cockpit-project:main Sep 29, 2023
98 checks passed
@martinpitt martinpitt deleted the apps-fix branch September 29, 2023 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Targetted for next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants