[PATCH 0/2] Fix mocking around networkxml2firewalltest

*** BLURB HERE *** Michal Prívozník (2): util: include virfirewall.h in virfirewalld.h virfirewallmock: Replace virFindFileInPath() with virFirewallDIsRegistered() src/util/virfirewalld.h | 2 ++ tests/virfirewallmock.c | 16 ++++------------ 2 files changed, 6 insertions(+), 12 deletions(-) -- 2.39.2

The virfirewalld.h file provides a declaration for virFirewallDApplyRule() which accepts an argument of type virFirewallLayer. But the typedef lives in virfirewall.h and thus including just virfirewalld.h is not sufficient. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virfirewalld.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virfirewalld.h b/src/util/virfirewalld.h index 004d10ec29..0e94d3507b 100644 --- a/src/util/virfirewalld.h +++ b/src/util/virfirewalld.h @@ -20,6 +20,8 @@ #pragma once +#include "virfirewall.h" + #define VIR_FIREWALL_FIREWALLD_SERVICE "org.fedoraproject.FirewallD1" typedef enum { -- 2.39.2

On Wed, May 03, 2023 at 11:00:54AM +0200, Michal Privoznik wrote:
The virfirewalld.h file provides a declaration for virFirewallDApplyRule() which accepts an argument of type virFirewallLayer. But the typedef lives in virfirewall.h and thus including just virfirewalld.h is not sufficient.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
--- src/util/virfirewalld.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/util/virfirewalld.h b/src/util/virfirewalld.h index 004d10ec29..0e94d3507b 100644 --- a/src/util/virfirewalld.h +++ b/src/util/virfirewalld.h @@ -20,6 +20,8 @@
#pragma once
+#include "virfirewall.h" + #define VIR_FIREWALL_FIREWALLD_SERVICE "org.fedoraproject.FirewallD1"
typedef enum { -- 2.39.2

Neither of tests that use virfirewallmock.c (networkxml2firewalltest, nwfilterebiptablestest, nwfilterxml2firewalltest, virfirewalltest) really call virFindFileInPath(). But at least networkxml2firewalltest calls virFirewallDIsRegistered(), under the hood. Now, the actual implementation connects to dbus and something, which is definitely not what we want in our test suite. Therefore, drop virFindFileInPath() implementation and provide implementation for virFirewallDIsRegistered() which just returns -2 to signal that firewalld is not registered. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virfirewallmock.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tests/virfirewallmock.c b/tests/virfirewallmock.c index 6b096701c9..793b954d87 100644 --- a/tests/virfirewallmock.c +++ b/tests/virfirewallmock.c @@ -17,18 +17,10 @@ #include <config.h> #include "internal.h" -#include "virfile.h" +#include "virfirewalld.h" -char * -virFindFileInPath(const char *file) +int +virFirewallDIsRegistered(void) { - if (file && - (g_strrstr(file, "ebtables") || - g_strrstr(file, "iptables") || - g_strrstr(file, "ip6tables"))) { - return g_strdup(file); - } - - /* We should not need any other binaries so return NULL. */ - return NULL; + return -2; } -- 2.39.2

On Wed, May 03, 2023 at 11:00:55AM +0200, Michal Privoznik wrote:
Neither of tests that use virfirewallmock.c (networkxml2firewalltest, nwfilterebiptablestest, nwfilterxml2firewalltest, virfirewalltest) really call virFindFileInPath(). But at least networkxml2firewalltest calls virFirewallDIsRegistered(), under the hood. Now, the actual implementation connects to dbus and something, which is
Is the "and something" a placeholder you forgot to replace? You can say "connects to dbus and lists the services" or even just "connects to dbus" is enough. The "something" seems weird there. With any of the versions applied Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

On Wed, May 3, 2023 at 11:01 AM Michal Privoznik <mprivozn@redhat.com> wrote:
*** BLURB HERE ***
Michal Prívozník (2): util: include virfirewall.h in virfirewalld.h virfirewallmock: Replace virFindFileInPath() with virFirewallDIsRegistered()
src/util/virfirewalld.h | 2 ++ tests/virfirewallmock.c | 16 ++++------------ 2 files changed, 6 insertions(+), 12 deletions(-)
-- 2.39.2
Reviewed-by: Kristina Hanicova <khanicov@redhat.com> Kristina
participants (3)
-
Kristina Hanicova
-
Martin Kletzander
-
Michal Privoznik