[PATCH 2/2] virfirewallmock: Replace virFindFileInPath() with virFirewallDIsRegistered()

Michal Privoznik posted 2 patches 1 year ago
[PATCH 2/2] virfirewallmock: Replace virFindFileInPath() with virFirewallDIsRegistered()
Posted by Michal Privoznik 1 year ago
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
Re: [PATCH 2/2] virfirewallmock: Replace virFindFileInPath() with virFirewallDIsRegistered()
Posted by Martin Kletzander 1 year ago
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>