DISCLAIMER: dnsmasq capabilities are empty as of v8.0.0-rc1~145.
In a real environment the dnsmasq capabilities are constructed
using dnsmasqCapsNewFromBinary(). We also have
dnsmasqCapsNewFromBuffer() to bypass checks that real code is
doing and just get capabilities object. The latter is used from
test suite.
However, with a little bit of mocking we can test the real life
code. All that's needed is to simulate dnsmasq's output for
--version and --help and mock a stat() that's done in
dnsmasqCapsRefreshInternal().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
tests/networkmock.c | 16 ++++++++++++++++
tests/networkxml2conftest.c | 38 ++++++++++++++++++++++++++++++++++++-
2 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/tests/networkmock.c b/tests/networkmock.c
index a9c13311a6..25014de8b8 100644
--- a/tests/networkmock.c
+++ b/tests/networkmock.c
@@ -28,3 +28,19 @@ virFindFileInPath(const char *file)
/* We should not need any other binaries so return NULL. */
return NULL;
}
+
+static int
+virMockStatRedirect(const char *path G_GNUC_UNUSED,
+ char **newpath G_GNUC_UNUSED)
+{
+ /* We don't need to redirect stat. Do nothing. */
+ return 0;
+}
+
+#define VIR_MOCK_STAT_HOOK \
+ if (strstr(path, "dnsmasq")) { \
+ memset(sb, 0, sizeof(*sb)); \
+ return 0; \
+ }
+
+#include "virmockstathelpers.c"
diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c
index 8a6657654a..68dd3023e1 100644
--- a/tests/networkxml2conftest.c
+++ b/tests/networkxml2conftest.c
@@ -12,6 +12,8 @@
#include "viralloc.h"
#include "network/bridge_driver.h"
#include "virstring.h"
+#define LIBVIRT_VIRCOMMANDPRIV_H_ALLOW
+#include "vircommandpriv.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -108,13 +110,47 @@ testCompareXMLToConfHelper(const void *data)
return result;
}
+static void
+buildCapsCallback(const char *const*args,
+ const char *const*env G_GNUC_UNUSED,
+ const char *input G_GNUC_UNUSED,
+ char **output,
+ char **error G_GNUC_UNUSED,
+ int *status,
+ void *opaque G_GNUC_UNUSED)
+{
+ if (STREQ(args[1], "--version")) {
+ *output = g_strdup("Dnsmasq version 2.67\n");
+ *status = EXIT_SUCCESS;
+ } else if (STREQ(args[1], "--help")) {
+ *output = g_strdup("--bind-dynamic\n--ra-param");
+ *status = EXIT_SUCCESS;
+ } else {
+ *status = EXIT_FAILURE;
+ }
+}
+
+static dnsmasqCaps *
+buildCaps(void)
+{
+ g_autoptr(dnsmasqCaps) caps = NULL;
+ g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew();
+
+ virCommandSetDryRun(dryRunToken, NULL, true, true, buildCapsCallback, NULL);
+
+ caps = dnsmasqCapsNewFromBinary();
+
+ return g_steal_pointer(&caps);
+}
+
+
static int
mymain(void)
{
int ret = 0;
g_autoptr(dnsmasqCaps) full = NULL;
- full = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.67\n--bind-dynamic\n--ra-param");
+ full = buildCaps();
#define DO_TEST(xname, xcaps) \
do { \
--
2.34.1
On Wed, Jan 12, 2022 at 09:47:56AM +0100, Michal Privoznik wrote:
> DISCLAIMER: dnsmasq capabilities are empty as of v8.0.0-rc1~145.
>
> In a real environment the dnsmasq capabilities are constructed
> using dnsmasqCapsNewFromBinary(). We also have
> dnsmasqCapsNewFromBuffer() to bypass checks that real code is
> doing and just get capabilities object. The latter is used from
> test suite.
>
> However, with a little bit of mocking we can test the real life
> code. All that's needed is to simulate dnsmasq's output for
> --version and --help and mock a stat() that's done in
> dnsmasqCapsRefreshInternal().
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> tests/networkmock.c | 16 ++++++++++++++++
> tests/networkxml2conftest.c | 38 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 53 insertions(+), 1 deletion(-)
This all works, but I wonder if we couldn't just create a trivial
shell script that behaves minimally the way we expect dnsmasq to, and
change our virFindFileInPath() mock so that it returns the absolute
path to it? That way we wouldn't need to implement any additional
mocking and the code would end up being much simpler. Diff below.
Also note that there is a pre-existing issue with the test, in that
we don't check that the value returned by dnsmasqCapsNewFrom*() is
non-NULL, and as a result if you change the version number in the
test string to something like 0.1 the test will still pass where it
definitely shouldn't.
diff --git a/tests/networkmock.c b/tests/networkmock.c
index a9c13311a6..dc1209a367 100644
--- a/tests/networkmock.c
+++ b/tests/networkmock.c
@@ -23,7 +23,7 @@ char *
virFindFileInPath(const char *file)
{
if (file && g_strrstr(file, "dnsmasq"))
- return g_strdup(file);
+ return g_strdup_printf("%s/virdnsmasqmock.sh", abs_srcdir);
/* We should not need any other binaries so return NULL. */
return NULL;
diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c
index 8a6657654a..fd2116756e 100644
--- a/tests/networkxml2conftest.c
+++ b/tests/networkxml2conftest.c
@@ -114,7 +114,7 @@ mymain(void)
int ret = 0;
g_autoptr(dnsmasqCaps) full = NULL;
- full = dnsmasqCapsNewFromBuffer("Dnsmasq version
2.67\n--bind-dynamic\n--ra-param");
+ full = dnsmasqCapsNewFromBinary();
#define DO_TEST(xname, xcaps) \
do { \
diff --git a/tests/virdnsmasqmock.sh b/tests/virdnsmasqmock.sh
new file mode 100755
index 0000000000..faaa94268c
--- /dev/null
+++ b/tests/virdnsmasqmock.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+case "$1" in
+ "--version")
+ echo "Dnsmasq version 2.67"
+ ;;
+ "--help")
+ echo "--bind-dynamic"
+ echo "--ra-param"
+ ;;
+ *)
+ exit 1
+ ;;
+esac
+
+exit 0
--
Andrea Bolognani / Red Hat / Virtualization
On 1/14/22 17:49, Andrea Bolognani wrote: > On Wed, Jan 12, 2022 at 09:47:56AM +0100, Michal Privoznik wrote: >> DISCLAIMER: dnsmasq capabilities are empty as of v8.0.0-rc1~145. >> >> In a real environment the dnsmasq capabilities are constructed >> using dnsmasqCapsNewFromBinary(). We also have >> dnsmasqCapsNewFromBuffer() to bypass checks that real code is >> doing and just get capabilities object. The latter is used from >> test suite. >> >> However, with a little bit of mocking we can test the real life >> code. All that's needed is to simulate dnsmasq's output for >> --version and --help and mock a stat() that's done in >> dnsmasqCapsRefreshInternal(). >> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >> --- >> tests/networkmock.c | 16 ++++++++++++++++ >> tests/networkxml2conftest.c | 38 ++++++++++++++++++++++++++++++++++++- >> 2 files changed, 53 insertions(+), 1 deletion(-) > > This all works, but I wonder if we couldn't just create a trivial > shell script that behaves minimally the way we expect dnsmasq to, and > change our virFindFileInPath() mock so that it returns the absolute > path to it? That way we wouldn't need to implement any additional > mocking and the code would end up being much simpler. Diff below. I thought that we should avoid shell for new contributions: https://libvirt.org/programming-languages.html > > Also note that there is a pre-existing issue with the test, in that > we don't check that the value returned by dnsmasqCapsNewFrom*() is > non-NULL, and as a result if you change the version number in the > test string to something like 0.1 the test will still pass where it > definitely shouldn't. Okay, let me address that in v3. Michal
On Mon, Jan 17, 2022 at 01:11:29PM +0100, Michal Prívozník wrote:
> On 1/14/22 17:49, Andrea Bolognani wrote:
> > This all works, but I wonder if we couldn't just create a trivial
> > shell script that behaves minimally the way we expect dnsmasq to, and
> > change our virFindFileInPath() mock so that it returns the absolute
> > path to it? That way we wouldn't need to implement any additional
> > mocking and the code would end up being much simpler. Diff below.
>
> I thought that we should avoid shell for new contributions:
>
> https://libvirt.org/programming-languages.html
Fair enough. Python version below.
#!/usr/bin/env python3
import sys
output = {
"--version": "Dnsmasq version 2.67",
"--help": "--bind-dynamic\n--ra-param",
}
if len(sys.argv) != 2 or sys.argv[1] not in output:
print("invalid usage")
sys.exit(1)
print(output[sys.argv[1]])
--
Andrea Bolognani / Red Hat / Virtualization
On 1/17/22 13:37, Andrea Bolognani wrote:
> On Mon, Jan 17, 2022 at 01:11:29PM +0100, Michal Prívozník wrote:
>> On 1/14/22 17:49, Andrea Bolognani wrote:
>>> This all works, but I wonder if we couldn't just create a trivial
>>> shell script that behaves minimally the way we expect dnsmasq to, and
>>> change our virFindFileInPath() mock so that it returns the absolute
>>> path to it? That way we wouldn't need to implement any additional
>>> mocking and the code would end up being much simpler. Diff below.
>>
>> I thought that we should avoid shell for new contributions:
>>
>> https://libvirt.org/programming-languages.html
>
> Fair enough. Python version below.
>
>
> #!/usr/bin/env python3
>
> import sys
>
> output = {
> "--version": "Dnsmasq version 2.67",
> "--help": "--bind-dynamic\n--ra-param",
> }
>
> if len(sys.argv) != 2 or sys.argv[1] not in output:
> print("invalid usage")
> sys.exit(1)
>
> print(output[sys.argv[1]])
>
And what exactly is the point? I'm failing to see why this would be any
better than mocking virCommand. Can you elaborate please?
Michal
On Mon, Jan 17, 2022 at 01:39:31PM +0100, Michal Prívozník wrote:
> On 1/17/22 13:37, Andrea Bolognani wrote:
> > On Mon, Jan 17, 2022 at 01:11:29PM +0100, Michal Prívozník wrote:
> >> On 1/14/22 17:49, Andrea Bolognani wrote:
> >>> This all works, but I wonder if we couldn't just create a trivial
> >>> shell script that behaves minimally the way we expect dnsmasq to, and
> >>> change our virFindFileInPath() mock so that it returns the absolute
> >>> path to it? That way we wouldn't need to implement any additional
> >>> mocking and the code would end up being much simpler. Diff below.
> >>
> >> I thought that we should avoid shell for new contributions:
> >>
> >> https://libvirt.org/programming-languages.html
> >
> > Fair enough. Python version below.
> >
> >
> > #!/usr/bin/env python3
> >
> > import sys
> >
> > output = {
> > "--version": "Dnsmasq version 2.67",
> > "--help": "--bind-dynamic\n--ra-param",
> > }
> >
> > if len(sys.argv) != 2 or sys.argv[1] not in output:
> > print("invalid usage")
> > sys.exit(1)
> >
> > print(output[sys.argv[1]])
>
> And what exactly is the point? I'm failing to see why this would be any
> better than mocking virCommand. Can you elaborate please?
I believe the diffstats speak for themselves :)
$ git diff --stat df09e45310..64325fa9ef
tests/networkmock.c | 16 ++++++++++++++++
tests/networkxml2conftest.c | 38 +++++++++++++++++++++++++++++++++++++-
2 files changed, 53 insertions(+), 1 deletion(-)
$ git diff --stat df09e45310..2b64fb492b
tests/networkmock.c | 2 +-
tests/networkxml2conftest.c | 2 +-
tests/virdnsmasqmock.py | 14 ++++++++++++++
3 files changed, 16 insertions(+), 2 deletions(-)
--
Andrea Bolognani / Red Hat / Virtualization
On 1/17/22 14:09, Andrea Bolognani wrote:
> On Mon, Jan 17, 2022 at 01:39:31PM +0100, Michal Prívozník wrote:
>> On 1/17/22 13:37, Andrea Bolognani wrote:
>>> On Mon, Jan 17, 2022 at 01:11:29PM +0100, Michal Prívozník wrote:
>>>> On 1/14/22 17:49, Andrea Bolognani wrote:
>>>>> This all works, but I wonder if we couldn't just create a trivial
>>>>> shell script that behaves minimally the way we expect dnsmasq to, and
>>>>> change our virFindFileInPath() mock so that it returns the absolute
>>>>> path to it? That way we wouldn't need to implement any additional
>>>>> mocking and the code would end up being much simpler. Diff below.
>>>>
>>>> I thought that we should avoid shell for new contributions:
>>>>
>>>> https://libvirt.org/programming-languages.html
>>>
>>> Fair enough. Python version below.
>>>
>>>
>>> #!/usr/bin/env python3
>>>
>>> import sys
>>>
>>> output = {
>>> "--version": "Dnsmasq version 2.67",
>>> "--help": "--bind-dynamic\n--ra-param",
>>> }
>>>
>>> if len(sys.argv) != 2 or sys.argv[1] not in output:
>>> print("invalid usage")
>>> sys.exit(1)
>>>
>>> print(output[sys.argv[1]])
>>
>> And what exactly is the point? I'm failing to see why this would be any
>> better than mocking virCommand. Can you elaborate please?
>
> I believe the diffstats speak for themselves :)
>
> $ git diff --stat df09e45310..64325fa9ef
> tests/networkmock.c | 16 ++++++++++++++++
> tests/networkxml2conftest.c | 38 +++++++++++++++++++++++++++++++++++++-
> 2 files changed, 53 insertions(+), 1 deletion(-)
>
> $ git diff --stat df09e45310..2b64fb492b
> tests/networkmock.c | 2 +-
> tests/networkxml2conftest.c | 2 +-
> tests/virdnsmasqmock.py | 14 ++++++++++++++
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
Alright, let's switch to python. I'm starting to not care about these
patches fate so I won't object.
Michal
On Mon, Jan 17, 2022 at 02:17:10PM +0100, Michal Prívozník wrote: > On 1/17/22 14:09, Andrea Bolognani wrote: > > On Mon, Jan 17, 2022 at 01:39:31PM +0100, Michal Prívozník wrote: > >> And what exactly is the point? I'm failing to see why this would be any > >> better than mocking virCommand. Can you elaborate please? > > > > I believe the diffstats speak for themselves :) > > > > $ git diff --stat df09e45310..64325fa9ef > > tests/networkmock.c | 16 ++++++++++++++++ > > tests/networkxml2conftest.c | 38 +++++++++++++++++++++++++++++++++++++- > > 2 files changed, 53 insertions(+), 1 deletion(-) > > > > $ git diff --stat df09e45310..2b64fb492b > > tests/networkmock.c | 2 +- > > tests/networkxml2conftest.c | 2 +- > > tests/virdnsmasqmock.py | 14 ++++++++++++++ > > 3 files changed, 16 insertions(+), 2 deletions(-) > > Alright, let's switch to python. I'm starting to not care about these > patches fate so I won't object. If you don't feel like doing that, you can consider your original patch Reviewed-by: Andrea Bolognani <abologna@redhat.com> and I'll try the script approach myself as a follow-up. -- Andrea Bolognani / Red Hat / Virtualization
© 2016 - 2026 Red Hat, Inc.