Always explicitly tear down pexpect instances and collect their
results. Assert on the results after orderly teardowns.
Track the current pexpect instance in test context so that it could be
still collected if the test blows up. That could provide more clue into
what went wrong in the test if it's due to a crash the testee.
Before:
[1573928.02238] <debug> config device C0:00:00:00:00:10: creating vlan connection for VLAN 700 on C0:00:00:00:00:10...
[1573928.02330] <debug> config device C0:00:00:00:00:10: connection "vlan2" (ac3c08f5-3e5c-38a3-a366-c16253de6db2) created
======================================================================
ERROR: test_oci_vlans (__main__.TestNmCloudSetup.test_oci_vlans)
----------------------------------------------------------------------
Traceback (most recent call last):
...
pexp.expect("some changes were applied for provider oci")
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
pexpect.exceptions.EOF: End Of File (EOF). Exception style platform.
After:
[1573928.02238] <debug> config device C0:00:00:00:00:10: creating vlan connection for VLAN 700 on C0:00:00:00:00:10...
[1573928.02330] <debug> config device C0:00:00:00:00:10: connection "vlan2" (ac3c08f5-3e5c-38a3-a366-c16253de6db2) created
*** pexpect'd process killed by SIGABRT ***
======================================================================
ERROR: test_oci_vlans (__main__.TestNmCloudSetup.test_oci_vlans)
----------------------------------------------------------------------
Traceback (most recent call last):
...
pexp.expect("some changes were applied for provider oci")
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
pexpect.exceptions.EOF: End Of File (EOF). Exception style platform.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2119
Allow running the following locally (for quick loval nm-c-s valgrind check),
without requiring $NM_TEST_CLIENT_NMCLI_PATH to be set.
$ NM_TEST_CLIENT_CLOUD_SETUP_PATH=build/src/nm-cloud-setup/nm-cloud-setup \
NMTST_USE_VALGRIND=1 python src/tests/client/test-client.py TestNmCloudSetup
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2119
The idea is to create a pair of VLAN and MACVLAN with AddAndActivate if
they are not present, and otherwise follow the ordinary (GetApplied &
Reapply) procedure if the devices are already present.
It attempts to modify attributes clearly belong to TestNmcli such as
_skip_test_for_l10n_diff or call methods that are in unittest.TestCase:
======================================================================
ERROR: test_002 (__main__.TestNmcli.test_002)
----------------------------------------------------------------------
Traceback (most recent call last):
File ".../src/tests/client/test-client.py", line 1508, in f
self.ctx.run_post()
~~~~~~~~~~~~~~~~~^^
File ".../src/tests/client/test-client.py", line 1185, in run_post
self.fail(
^^^^^^^^^
AttributeError: 'NMTestContext' object has no attribute 'fail'
It has presumably been moved out of TestNmcli at some point, but that
seems to have been in error, as it's also pretty specific to the nmcli
test cases. Not useful for cloud-init tests that also utilize
NMTestContext. Move it back.
- test for "-order" option with `nmcli connection show`.
- test for order of activated devices. Optimally, the devices
should be in activating vs. activated state. I fail to do that,
the mock implementation is cumbersome to use. It still seems useful
to have this (maybe it could be improved).
f-string is not supported in python2, and the autotool build complains
about it as follows:
```
LIBTOOL="/bin/sh ./libtool" "../src/tests/client/test-client.sh" "." ".." "python2" -- TestNmCloudSetup
File "/builds/NetworkManager/NetworkManager/src/tests/client/test-client.py", line 722
return f"{major}.{minor}.{micro}"
^
SyntaxError: invalid syntax
test-client.py failed!!
make[3]: *** [check-local-tests-client] Error 1
File "/builds/NetworkManager/NetworkManager/src/tests/client/test-client.py", line 722
return f"{major}.{minor}.{micro}"
^
SyntaxError: invalid syntax
test-client.py failed!!
```
Also, python2 complains about extra comma during argument unpacking.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1718
When updating NetworkManager to a new version, normally the service is
not restarted by the installer to avoid interrupting networking.
However, next nmcli invocation will use the updated version, but against
the older version of the daemon that is still running. Although this is
suposed to work, it is advisable that nmcli and daemon's versions are
the same. Emit a warning recommending restarting the daemon.
Add nmcli test to check the new feature. To avoid breaking the existing
tests, test-networkmanager-service now reports the same version than the
running nmcli except if it's instructed to report a different one.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1703
(cherry picked from commit fb851f3294)
If --offline and --ask were used at the same time, and endless loop
showing the readline's prompt but without waiting for user's input
happened.
This was because when using --offline, all arguments are parsed and
resolved before running the g_main_loop. In nmc_readline_helper it was
checked that the main loop is running, so if g_main_loop_quit is called
we can stop waiting for user's input.
Fix this bug by continue polling for user input if the main loop is
running or if we are in offline mode. Cancelling the user input is
still possible both in normal and offline mode with Ctrl+C or Ctrl+D.
Added a test case to verify that this still works after future changes.
With the unit test framework, we define special methods, like setUp()
and test_*(). This is documented, but not obvious.
Previously, TestNmClient was the base class for our tests classes, and
it provided some functionality (and state). It was utterly confusing how
pieces fit together.
Instead, move the state to a new class NMTestContext(). That contains
most of the code from TestNmClient. Drop TestNmClient and let the test
classes directly descend from unittest.TestCase.
The difference is, when you now look at a certain test (test_001()), you
can easier understand which code runs when. First, the test class has a
setUp() method which runs, but that method is now trivial without extra
context. Second, there is the @nm_test attribute that wraps the
function. But that's it. It's all at one place, and we delegate instead
of inherit.
Don't rely on resources provided by mock metadata server by default,
create the from within the test instead.
This allows for more flexibility, but the locality of the test fixture
relative to the tests makes the test more legible.
When a pexpect check fails, we want to see the full content of the
buffer, so we can better see where it went wrong. Increase the context
that is printed in the error message.
"preexec_fn" is not great, because it is not generally safe in multi
threaded code (and we don't know whether the test didn't start other
threads already, like a GDBus worker thread). Well, it probably is safe,
if you only call async signal safe code, but it's not clear what os.dup2()
does under the hood.
Just avoid that. We can pass on the FD directly.
test_ec2 (__main__.TestNmCloudSetup.test_ec2) ... /usr/lib64/python3.11/unittest/case.py:579: ResourceWarning: unclosed file <_io.BufferedWriter name=5>
if method() is not None:
ResourceWarning: Enable tracemalloc to get the object allocation traceback
ok
Fixes: d89d42bf23 ('tests/client: test nm-cloud-setup')
This seems unnecessary, because we spawn the child process via subprocess.Popen and
set "pass_fds". That already ensures to pass on the FD.
Worse, socket.set_inheritable() is only added in Python 3.4, that means the
test is gonna break for Python 3.2 and 3.3. Just avoid that by not using the
unnecessary function. For the same reason, drop "inheritable=True" from
os.dup2(). "inheritable=True" is already the default, but only exists
since Python 3.4.
Fixes: d89d42bf23 ('tests/client: test nm-cloud-setup')
The test uses subprocess.Popen()'s "pass_fd" argument. That is only
available since Python 3.2. Possibly it could be solved differently, but
that is not implemented. Instead, skip the test.
Also, socket.socket.set_inheritable() is Python 3.4. But presumably
we don't need it.
Fixes: d89d42bf23 ('tests/client: test nm-cloud-setup')
The mock service is more widely useful -- in particular for testing
nm-cloud-setup in a following commit.
Split the commonly useful parts into TestNmClient class.
During srv_shutdown() we do
p.stdin.close()
p.kill()
Usually, the kill wins and the service just drops off the bus:
libnm-dbus[3201919]: <debug> [438617.45324] nmclient[40f7938626f3f5f0]: name owner changed: ":1.1" -> (null)
libnm-dbus[3201919]: <debug> [438617.45332] nmclient[40f7938626f3f5f0]: release all
at which point all objects in NMClient get destroyed and the signals get
emitted in the order:
libnm-dbus[3201919]: <trace> [438617.45574] nmclient[40f7938626f3f5f0]: [nmclient] emit "device-removed" signal for /org/freedesktop/NetworkManager/Devices/1
nmcli[out]: eth0: device removed
libnm-dbus[3201919]: <trace> [438617.45590] nmclient[40f7938626f3f5f0]: [nmclient] emit "any-device-removed" signal for /org/freedesktop/NetworkManager/Devices/1
libnm-dbus[3201919]: <trace> [438617.45593] nmclient[40f7938626f3f5f0]: [nmclient] emit "connection-removed" signal for /org/freedesktop/NetworkManager/Settings/Connectio>
nmcli[out]: con-1: connection profile removed
However, sometimes the stub service notices that stdin was closed and it
sends signals about shutting down:
libnm-dbus[3201061]: <trace> [438226.44965] nmclient[401639659459c316]: interfaces-removed: [/org/freedesktop/NetworkManager/Settings] receive interface remove event for >
libnm-dbus[3201061]: <trace> [438226.44966] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager/Settings]: changed-type 0x01 linked
libnm-dbus[3201061]: <trace> [438226.44967] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager/Settings]: changed-type 0x01 consumed
libnm-dbus[3201061]: <trace> [438226.44968] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager/Settings]: changed-type 0x02 linked
libnm-dbus[3201061]: <trace> [438226.44969] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager/Settings]: unregister NMClient from D-Bus object
libnm-dbus[3201061]: <trace> [438226.44971] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager/Settings]: drop D-Bus instance
libnm-dbus[3201061]: <trace> [438226.44971] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager/Settings]: set D-Bus object state unlinked
libnm-dbus[3201061]: <trace> [438226.44972] nmclient[401639659459c316]: [nmclient] emit "connection-removed" signal for /org/freedesktop/NetworkManager/Settings/Connectio>
nmcli[out]: con-1: connection profile removed
libnm-dbus[3201061]: <trace> [438226.44992] nmclient[401639659459c316]: interfaces-removed: [/org/freedesktop/NetworkManager] receive interface remove event for interface>
libnm-dbus[3201061]: <trace> [438226.44994] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager]: changed-type 0x01 linked
libnm-dbus[3201061]: <trace> [438226.44995] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager]: changed-type 0x01 consumed
libnm-dbus[3201061]: <trace> [438226.44996] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager]: changed-type 0x02 linked
libnm-dbus[3201061]: <trace> [438226.44998] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager]: unregister NMClient from D-Bus object
libnm-dbus[3201061]: <trace> [438226.44999] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager]: drop D-Bus instance
libnm-dbus[3201061]: <trace> [438226.45000] nmclient[401639659459c316]: [/org/freedesktop/NetworkManager]: set D-Bus object state unlinked
libnm-dbus[3201061]: <trace> [438226.45001] nmclient[401639659459c316]: [nmclient] emit "device-removed" signal for /org/freedesktop/NetworkManager/Devices/1
nmcli[out]: eth0: device removed
libnm-dbus[3201061]: <trace> [438226.45005] nmclient[401639659459c316]: [nmclient] emit "any-device-removed" signal for /org/freedesktop/NetworkManager/Devices/1
nmcli[out]: NetworkManager is stopped
libnm-dbus[3201061]: <debug> [438226.45545] nmclient[401639659459c316]: name owner changed: ":1.1" -> (null)
libnm-dbus[3201061]: <debug> [438226.45550] nmclient[401639659459c316]: release all
The fix is to accept the events in any order.
(cherry picked from commit 8548ab29ee)
The test stub service watches stdin, and if it gets closed the service
will shut down. Note that the service does not catch any signals, so
sending a signal will kill the service right away.
The previous code first closed stdin, and then killed the process.
That can result in different outcomes on D-Bus. Usually the signal
gets received first, and the test service just drops off the bus.
Sometimes it notices the closing of stdin and shuts actively down.
That can make a difference, especially for the test_monitor() test which
runs the monitor while stopping the service.
We could just always kill the stub service to get consistent behavior.
However, that doesn't seem very useful. Instead, randomize the behavior
to easier see how the behavior differs.
(cherry picked from commit fc282d5e05)
For debugging libnm, LIBNM_CLIENT_DEBUG can be very useful.
As the tests compare stdout/stderr from nmcli with expected output, just
enabling it will break the tests. However, in combination with
LIBNM_CLIENT_DEBUG_FILE this can be very useful.
Preserve and pass on the environment variables, if set.
(cherry picked from commit 1630009234)
Sort imports by name. Also avoid "from signal import SIGINT". I find
it ugly to import names in the current namespace. "SIGINT" should be
refered to by its full name, including the package/namespace.
(cherry picked from commit ee17346cee)
This will allow to find some memory leaks and memory corruptions.
The bulk of the nmcli calls are still not hooked up with valgrind.
Since we call nmcli a thousand time, we could not just run valgrind with
all of them. We would have instead to enable it randomly. This is
more work.
(cherry picked from commit debf78dbed)
