This would only be necessary when editing the test themselves,
to not only remove individual tests, but entire test runs.
It's not necessary to describe that in the howto.
Otherwise, we just see "returncode: -11", which isn't very clear.
By default, our test nmcli invocations should never exit by a signal.
Usually, when we encounter a signal, it indicates to a bug and a crash
during the test.
Print more information about the exit code.
returncode: -11 (SIGNAL SIGSEGV)
This will be necessary to sort a list of text that contains text to
replace. The use case is to get all UUIDs from the server, but some
of these UUIDs are dynamic. So, in order to have a fixed sort order,
we need to handle sorting the list, based on the replacement.
(cherry picked from commit 347253be2f)
We want to issue a nmcli command with the connection's UUID in the command
line. As the command line is stored to disk and compared, we cannot use UUIDs
that are randomly generated. Instead, we need to support replacing the text
in the command line.
(cherry picked from commit 7d048f1a5b)
The variable name should say what it is, not what it is used for.
E.g. we would use the very same replacements for stderr. Rename
so that it can (by its name) used for different purposes.
(cherry picked from commit ffc235c30d)
We need to actually read the stdout/stderr of the nmcli programs.
Otherwise, the pipe might fill uup and block to process (eventually
leading to timeout).
Debugging tests that are called by test-client.py is cumbersome.
One way would be to set NM_TEST_CLIENT_NMCLI_PATH to a wrapper script.
However, then we want to know from the wrapper script which test
we are currently calling. Add that to the environment.
The default timeout is really only here to abort the test when it
definitely failed to avoid keeping it hanging forever. It's not part
of any strict assertions and should be large.
Fixes: bb4b749595 ('clients/tests: don't wait for first job before scheduling parallel jobs')
Previously, the test would kick off 15 processes in parallel, but
the first job in the queue would block more processes from being
started.
That is, async_start() would only start 15 processes, but since none of
them were reaped before async_wait() was called, no more than 15 jobs
were running during the start phase. That is not a real issue, because
the start phase is non-blocking and queues all the jobs quickly. It's
not really expected that during that time many processes already completed.
Anyway, this was a bit ugly.
The bigger problem is that async_wait() would always block for the
first job to complete, before starting more processes. That means,
if the first job in the queue takes unusually long, then this blocks
other processes from getting reaped and new processes from being
started.
Instead, don't block only one one jobs, but poll them in turn for a
short amount of time. Whichever process exits first will be completed
and more jobs will be started.
In fact, in the current setup it's hard to notice any difference,
because all nmcli invocations take about the same time and are
relatively fast. That this approach parallelizes better can be seen
when the runtime of jobs varies stronger (and some invocations take
a notably longer time). As we later want to run nmcli under valgrind,
this probably will make a difference.
An alternative would be not to poll()/wait() for child processes,
but somehow get notified. For example, we could use a GMainContext
and watch child processes. But that's probably more complicated
to do, so let's keep the naive approach with polling.
If we failed on process wait, we didn't close the pipes and no clear output
of what happened was exposed.
So use a finally stanza to close the pipes and print stdout and stderr
on failure.
The client tests compare the test output with a .expected file that is
commit to git and that contains the expected output.
The expected output contains data like
size: 395
location: clients/tests/test-client.py:842:test_001()/1
cmd: $NMCLI
lang: C
returncode: 0
stdout: 277 bytes
>>>
...
Note that there is the line number (clients/tests/test-client.py:842) of
the source code where nmcli is called. This is to help correlate the output
with the test code.
However, Python 3.8 changes behavior and for function calls that span multiple
lines, frame.f_lineno will give now the starting line (previously, it gave the last
line) (see [1]).
No longer include the line number, as it is not stable accross Python versions.
If you really care, you can set NM_TEST_WITH_LINENO to get the line numbers back.
Of course, then the expected output won't match anymore, and you'd have to regenerate
it first. This is only useful if you debug tests, and want to have it easier to
correlate output with the tests while developing them.
[1] https://bugs.python.org/issue38283https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/292
It's unclear how to workaround this issue, so that the tests
work with older python versions and 3.8-beta.
Let's wait whether this will really be released as 3.8 and
for now just skip the test.
Python 3 doesn't like this:
======================================================================
ERROR: test_001 (__main__.TestNmcli)
----------------------------------------------------------------------
Traceback (most recent call last):
File "./clients/tests/test-client.py", line 785, in f
self._nm_test_post()
File "./clients/tests/test-client.py", line 767, in _nm_test_post
content_new = ''.join([r['content'] for r in results])
TypeError: sequence item 0: expected str instance, bytes found
Recent Python versions warn about this:
./clients/tests/test-client.py:569: SyntaxWarning: "is" with a literal. Did you mean "=="?
elif lang is de:
./clients/tests/test-client.py:572: SyntaxWarning: "is" with a literal. Did you mean "=="?
elif lang is pl:
And rightly so: https://bugs.python.org/issue34850
Instead of straight-out rejecting extra parameters for various nmcli
sub-commands (such as "nmcli dev status", "nmcli dev wifi rescan" or
"nmcli dev wifi connect", etc.), we just print a warning and go ahead.
This is unhelpful. In case the user makes a typo, we'll do the wrong
thing and possibly even drown the warning in the output.
While at that, let's make the error message consistent. One less string
to translate.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/217
I saw this timeout reached in our gitlab-ci. I think it was due to the machine
being busy and taking more than 2 seconds. Assuming the timeout was just too short,
increase it to 4 seconds.
"ipv4.dns-options" and "ipv6.dns-options" are special, in that they can
be either unset/default or an empty list of options. That means, nmcli
must treat these two options differently.
For the (terse) getter, it returns "" for default and " " for empty.
The setter must likewise support and distingish between these two cases.
Cleanup the handling of such options. This only applies to properties of
type "multilist". Hence, add multilist.clear_emptyunset_fcn() handler
that indicates that the property is of that kind.
Try:
nmcli connection modify "$PROFILE" ipv4.dns-options ''
nmcli connection modify "$PROFILE" ipv4.dns-options ' '
and compare the output:
nmcli connection show "$PROFILE" | sed -n '/ipv4.dns-options/ s/.*/<\0>/p'
nmcli -t connection show "$PROFILE" | sed -n '/ipv4.dns-options/ s/.*/<\0>/p'
nmcli -o connection show "$PROFILE" | sed -n '/ipv4.dns-options/ s/.*/<\0>/p'
Leak detection adds unhelpful messages to the stderr of nmcli, making
tests fail. For example:
=================================================================
==17156==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 256 byte(s) in 2 object(s) allocated from:
#0 0x7f08c7e27c88 in realloc (/lib64/libasan.so.5+0xefc88)
#1 0x7f08c7546e7d in g_realloc (/lib64/libglib-2.0.so.0+0x54e7d)
On failure the self.fail() message often appears in the middle of the
diff between expected/actual output, making it hard to read. Since
print() output goes to stdout (which is buffered) and self.fail() to
stderr (which is not), flush stdout before printing the failure
message to ensure the two don't mix.
Instead of re-fetching the UUID each time with a D-Bus call,
cache/memoize it.
On my system, this improves
$ make check-local-clients-tests-test-client
from 20.9 to 20.4 seconds (- 2.6%). That is not stellar, but noticible
enough to warrant this simple change.
These nmcli calls are inside a Util.iter_nmcli_output_modes()
loop, hence, --terse will passed to them already.
Specifying --terse more than once, causes nmcli to fail.
We didn't actually want to test how nmcli rejects such
duplicate arguments. Adjust the test.
Add more tests for various output modes of nmcli.
This most interestingly includes a terse output for
$ nmcli device status
Which was not tested previously (but a later commit will change
behavior here).
This blows up the number of tests even further.
Previously, the test invoked nmcli 850 times (taking ~15 seconds
ony my machine), afterwards, it is 1334 times (taking ~20 seconds).
No doubt, this seems excessive and questionable.
However, I maintain that the computer doesn't mind running a lot of
redundant tests. The important thing is, that we cover as many cases
as possible. Trying at the same time to minimize the number of tests
by avoiding duplicates and redundant tests, is just a lot of manual
labor. Manual labor that must be repeated whenever new tests are added, to
ensure that this test case is not yet covered. So, I am fine
with the large number of tests. Let the computer do the work.
Instead of letting each nmcli run write an individual .expected file,
combine the output of multiple runs in one file (per test).
Advantages:
- since there is a very large number of tests, having a file for each
tests is cumbersome. For example, since they are all added to
$(EXTRA_DIST) (and since we use non-recursive make), autoconf can
easily hit a length limit when processing all the file names.
- previously, whenever we added tests, all .expected files shifted
and the diff was large. Now, there is a chance that the diff is
smaller and more accurate.
Commands that fail with G_DEBUG=fatal-warnings produce unstable
output like
(process:10743): GLib-CRITICAL **: 16:29:13.635: g_ptr_array_add: assertion 'rarray' failed
To workaround that, add a new option for marking the output as unstable.
An alternative might be to extend the replace_stdout, replace_stderr
arguments to support more powerful matching, like by specifying regular
expression for replacing. However, it's just too complicated. Add a simpler
workaround by passing _UNSTABLE_OUTPUT.
