cockpit-project / cockpit

Cockpit is a web-based graphical interface for servers.
http://www.cockpit-project.org/
GNU Lesser General Public License v2.1
11.2k stars 1.11k forks source link

test: Terminate sessions before restoring files/directories #20450

Closed martinpitt closed 5 months ago

martinpitt commented 5 months ago

Most of our tests don't bother logging out at the end, they rely on our generic nondestructive setup. But this causes some noise when using restore_dir() and friends, e.g. in TestApps.testBasic:

+ rm -rf /usr/share/metainfo; mv /var/lib/cockpittest/_usr_share_metainfo /usr/share/metainfo
+ mv /var/lib/cockpittest/_var_lib_PackageKit_transactions.db /var/lib/PackageKit/transactions.db
> warning: can't remove watch: Invalid argument
can't add watch for <ctypes.c_char_Array_20 object at 0x7f9aed107750>: Permission denied
Traceback (most recent call last):
  File "<string>", line 445, in <module>
  File "<string>", line 442, in watch_db
[...]
  File "<string>", line 148, in watch_directory
PermissionError: [Errno 13] Permission denied: '/usr/share/metainfo'

As the tests themselves call restore_dir() (which happens after the generic nonDestructiveSetup(), the restoration also happens before nonDestructiveSetup() can kill all running sessions. Thus the cockpit page, bridge etc. are still active then and keep messing around with the file system.

This don't actually break the test, but errors like the above are confusing when debugging and staring at test output.

To clean this up, move the session termination into tearDown(), which runs first after the test method ends -- in particular, before the addCleanup() handlers. Move the previously nested terminate_sessions() helper function to a private method of MachineCase, so that tearDown() does not get too long.

This removes the need to daemon-reload the admin user session in TestService's cleanup -- at that point there are no user sessions at all any more.


This should fix #20446 as far as I understand it. If not, it at least gets rid of some unrelated noise.

martinpitt commented 5 months ago

Reproducer:

test/verify/check-system-services TestServices.testTransientUnits $RUNC -stv
martinpitt commented 5 months ago

I'm adding the kpatch logging fix here. It's too small to warrant its own PR, and it will help us to track down that bug (if it even still happens after this).

martinpitt commented 5 months ago

I'll pull this into all of our projects to validate that this works everywhere.