Unverified Commit 7b803c35 authored by Cosima Neidahl's avatar Cosima Neidahl Committed by GitHub
Browse files

lomiri.lomiri-indicator-network: 1.1.0 -> 1.1.1 (#412649)

parents 657a9316 9dba3a34
Loading
Loading
Loading
Loading
+0 −240
Original line number Diff line number Diff line
From 9c2a6a6349f705017e3c8a34daa4ba1805586498 Mon Sep 17 00:00:00 2001
From: OPNA2608 <opna2608@protonmail.com>
Date: Thu, 30 Jan 2025 14:53:02 +0100
Subject: [PATCH] tests/unit/secret-agent/test-secret-agent: Make sure signal
 emitted on agent startup doesn't leak into tests

---
 tests/unit/secret-agent/test-secret-agent.cpp | 116 ++++++++++--------
 1 file changed, 67 insertions(+), 49 deletions(-)

diff --git a/tests/unit/secret-agent/test-secret-agent.cpp b/tests/unit/secret-agent/test-secret-agent.cpp
index 1f1cd7e9..9c72e251 100644
--- a/tests/unit/secret-agent/test-secret-agent.cpp
+++ b/tests/unit/secret-agent/test-secret-agent.cpp
@@ -29,6 +29,16 @@
 #include <lomiri/gmenuharness/MatchUtils.h>
 #include <lomiri/gmenuharness/MenuMatcher.h>
 
+#define WAIT_FOR_SIGNALS(signalSpy, signalsExpected)\
+{\
+	while (signalSpy.size() < signalsExpected)\
+	{\
+		ASSERT_TRUE(signalSpy.wait()) << "Waiting for " << signalsExpected << " signals, got " << signalSpy.size();\
+	}\
+	ASSERT_EQ(signalsExpected, signalSpy.size()) << "Waiting for " << signalsExpected << " signals, got " << signalSpy.size();\
+}
+
+
 using namespace std;
 using namespace testing;
 using namespace QtDBusTest;
@@ -49,21 +59,6 @@ protected:
 		dbusMock.registerTemplate(NM_DBUS_SERVICE, NETWORK_MANAGER_TEMPLATE_PATH, {}, QDBusConnection::SystemBus);
 		dbusTestRunner.startServices();
 
-		QProcessEnvironment env(QProcessEnvironment::systemEnvironment());
-		env.insert("SECRET_AGENT_DEBUG_PASSWORD", "1");
-		secretAgent.setProcessEnvironment(env);
-		secretAgent.setReadChannel(QProcess::StandardOutput);
-		secretAgent.setProcessChannelMode(QProcess::ForwardedErrorChannel);
-		secretAgent.start(SECRET_AGENT_BIN, QStringList() << "--print-address");
-		secretAgent.waitForStarted();
-		secretAgent.waitForReadyRead();
-		agentBus = secretAgent.readAll().trimmed();
-
-		agentInterface.reset(
-				new OrgFreedesktopNetworkManagerSecretAgentInterface(agentBus,
-				NM_DBUS_PATH_SECRET_AGENT, dbusTestRunner.systemConnection()));
-
-
 		notificationsInterface.reset(
 				new OrgFreedesktopDBusMockInterface(
 						"org.freedesktop.Notifications",
@@ -72,8 +67,11 @@ protected:
 	}
 
 	virtual ~TestSecretAgentCommon() {
-		secretAgent.terminate();
-		secretAgent.waitForFinished();
+		if (secretAgent.state() != QProcess::NotRunning)
+		{
+				secretAgent.terminate();
+				secretAgent.waitForFinished();
+		}
 	}
 
 	QVariantDictMap connection(const QString &keyManagement) {
@@ -111,6 +109,32 @@ protected:
 		return connection;
 	}
 
+	void setupSecretAgent (void) {
+		QSignalSpy notificationSpy(notificationsInterface.data(),
+				SIGNAL(MethodCalled(const QString &, const QVariantList &)));
+
+		QProcessEnvironment env(QProcessEnvironment::systemEnvironment());
+		env.insert("SECRET_AGENT_DEBUG_PASSWORD", "1");
+		secretAgent.setProcessEnvironment(env);
+		secretAgent.setReadChannel(QProcess::StandardOutput);
+		secretAgent.setProcessChannelMode(QProcess::ForwardedErrorChannel);
+		secretAgent.start(SECRET_AGENT_BIN, QStringList() << "--print-address");
+		secretAgent.waitForStarted();
+		secretAgent.waitForReadyRead();
+
+		agentBus = secretAgent.readAll().trimmed();
+
+		agentInterface.reset(
+				new OrgFreedesktopNetworkManagerSecretAgentInterface(agentBus,
+				NM_DBUS_PATH_SECRET_AGENT, dbusTestRunner.systemConnection()));
+
+		WAIT_FOR_SIGNALS(notificationSpy, 1);
+		{
+				const QVariantList &call(notificationSpy.at(0));
+				EXPECT_EQ(call.at(0), "GetServerInformation");
+		}
+	}
+
 	DBusTestRunner dbusTestRunner;
 
 	DBusMock dbusMock;
@@ -163,22 +187,21 @@ static void transform(QVariantList &list) {
 }
 
 TEST_P(TestSecretAgentGetSecrets, ProvidesPasswordForWpaPsk) {
+	setupSecretAgent();
+
+	QSignalSpy notificationSpy(notificationsInterface.data(),
+			SIGNAL(MethodCalled(const QString &, const QVariantList &)));
+
 	QDBusPendingReply<QVariantDictMap> reply(
 			agentInterface->GetSecrets(connection(GetParam().keyManagement),
 					QDBusObjectPath("/connection/foo"),
 					SecretAgent::NM_WIRELESS_SECURITY_SETTING_NAME, QStringList(),
 					5));
 
-	QSignalSpy notificationSpy(notificationsInterface.data(),
-	SIGNAL(MethodCalled(const QString &, const QVariantList &)));
-	if (notificationSpy.empty())
-	{
-		ASSERT_TRUE(notificationSpy.wait());
-	}
+	WAIT_FOR_SIGNALS(notificationSpy, 1);
 
-	ASSERT_EQ(1, notificationSpy.size());
 	const QVariantList &call(notificationSpy.at(0));
-	EXPECT_EQ("Notify", call.at(0).toString().toStdString());
+	EXPECT_EQ("Notify", call.at(0));
 
 	QVariantList args(call.at(1).toList());
 	transform(args);
@@ -254,6 +277,7 @@ class TestSecretAgent: public TestSecretAgentCommon, public Test {
 };
 
 TEST_F(TestSecretAgent, GetSecretsWithNone) {
+	setupSecretAgent();
 
 	QDBusPendingReply<QVariantDictMap> reply(
 			agentInterface->GetSecrets(
@@ -272,6 +296,8 @@ TEST_F(TestSecretAgent, GetSecretsWithNone) {
 /* Tests that if we request secrets and then cancel the request
    that we close the notification */
 TEST_F(TestSecretAgent, CancelGetSecrets) {
+	setupSecretAgent();
+
 	QSignalSpy notificationSpy(notificationsInterface.data(), SIGNAL(MethodCalled(const QString &, const QVariantList &)));
 
 	agentInterface->GetSecrets(
@@ -280,23 +306,19 @@ TEST_F(TestSecretAgent, CancelGetSecrets) {
 			SecretAgent::NM_WIRELESS_SECURITY_SETTING_NAME, QStringList(),
 			5);
 
-	notificationSpy.wait();
-
-	ASSERT_EQ(1, notificationSpy.size());
-	const QVariantList &call(notificationSpy.at(0));
-	EXPECT_EQ("Notify", call.at(0).toString().toStdString());
+	WAIT_FOR_SIGNALS(notificationSpy, 1);
+	{
+		const QVariantList &call(notificationSpy.at(0));
+		EXPECT_EQ("Notify", call.at(0));
+	}
 
 	notificationSpy.clear();
 
 	agentInterface->CancelGetSecrets(QDBusObjectPath("/connection/foo"),
 			SecretAgent::NM_WIRELESS_SECURITY_SETTING_NAME);
 
-	if (notificationSpy.empty())
-	{
-		ASSERT_TRUE(notificationSpy.wait());
-	}
+	WAIT_FOR_SIGNALS(notificationSpy, 1);
 
-	ASSERT_EQ(1, notificationSpy.size());
 	const QVariantList &closecall(notificationSpy.at(0));
 	EXPECT_EQ("CloseNotification", closecall.at(0).toString().toStdString());
 }
@@ -304,6 +326,8 @@ TEST_F(TestSecretAgent, CancelGetSecrets) {
 /* Ensures that if we request secrets twice we close the notification
    for the first request */
 TEST_F(TestSecretAgent, MultiSecrets) {
+	setupSecretAgent();
+
 	QSignalSpy notificationSpy(notificationsInterface.data(), SIGNAL(MethodCalled(const QString &, const QVariantList &)));
 
 	agentInterface->GetSecrets(
@@ -312,15 +336,12 @@ TEST_F(TestSecretAgent, MultiSecrets) {
 			SecretAgent::NM_WIRELESS_SECURITY_SETTING_NAME, QStringList(),
 			5);
 
-	if (notificationSpy.empty())
+	WAIT_FOR_SIGNALS(notificationSpy, 1);
 	{
-		ASSERT_TRUE(notificationSpy.wait());
+		const QVariantList &call(notificationSpy.at(0));
+		EXPECT_EQ("Notify", call.at(0));
 	}
 
-	ASSERT_EQ(1, notificationSpy.size());
-	const QVariantList &call(notificationSpy.at(0));
-	EXPECT_EQ("Notify", call.at(0).toString().toStdString());
-
 	notificationSpy.clear();
 
 	agentInterface->GetSecrets(
@@ -329,14 +350,7 @@ TEST_F(TestSecretAgent, MultiSecrets) {
 			SecretAgent::NM_WIRELESS_SECURITY_SETTING_NAME, QStringList(),
 			5);
 
-	if (notificationSpy.empty())
-	{
-		ASSERT_TRUE(notificationSpy.wait());
-	}
-	if (notificationSpy.size() == 1)
-	{
-		ASSERT_TRUE(notificationSpy.wait());
-	}
+	WAIT_FOR_SIGNALS(notificationSpy, 2);
 
 	ASSERT_EQ(2, notificationSpy.size());
 	const QVariantList &closecall(notificationSpy.at(1));
@@ -347,11 +361,15 @@ TEST_F(TestSecretAgent, MultiSecrets) {
 }
 
 TEST_F(TestSecretAgent, SaveSecrets) {
+	setupSecretAgent();
+
 	agentInterface->SaveSecrets(QVariantDictMap(),
 			QDBusObjectPath("/connection/foo")).waitForFinished();
 }
 
 TEST_F(TestSecretAgent, DeleteSecrets) {
+	setupSecretAgent();
+
 	agentInterface->DeleteSecrets(QVariantDictMap(),
 			QDBusObjectPath("/connection/foo")).waitForFinished();
 }
-- 
2.47.1
+5 −12
Original line number Diff line number Diff line
@@ -33,13 +33,13 @@

stdenv.mkDerivation (finalAttrs: {
  pname = "lomiri-indicator-network";
  version = "1.1.0";
  version = "1.1.1";

  src = fetchFromGitLab {
    owner = "ubports";
    repo = "development/core/lomiri-indicator-network";
    tag = finalAttrs.version;
    hash = "sha256-pN5M5VKRyo6csmI/vrmp/bonnap3oEdPuHAUJ1PjdOs=";
    hash = "sha256-R5W1MmT+H9i8NXrzOv2xaVu8TKPCRCAAswwM/tflkQ0=";
  };

  outputs = [
@@ -48,20 +48,11 @@ stdenv.mkDerivation (finalAttrs: {
    "doc"
  ];

  patches = [
    ./1001-test-secret-agent-Make-GetServerInformation-not-leak-into-tests.patch
  ];

  postPatch = ''
    # Override original prefixes
    substituteInPlace data/CMakeLists.txt \
      --replace-fail 'pkg_get_variable(DBUS_SESSION_BUS_SERVICES_DIR dbus-1 session_bus_services_dir)' 'pkg_get_variable(DBUS_SESSION_BUS_SERVICES_DIR dbus-1 session_bus_services_dir DEFINE_VARIABLES datadir=''${CMAKE_INSTALL_FULL_SYSCONFDIR})' \
      --replace-fail 'pkg_get_variable(SYSTEMD_USER_DIR systemd systemduserunitdir)' 'pkg_get_variable(SYSTEMD_USER_DIR systemd systemduserunitdir DEFINE_VARIABLES prefix=''${CMAKE_INSTALL_PREFIX})'

    # Fix typo
    # Remove when https://gitlab.com/ubports/development/core/lomiri-indicator-network/-/merge_requests/131 merged & in release
    substituteInPlace src/indicator/nmofono/wwan/modem.cpp \
      --replace-fail 'if (m_isManaged = managed)' 'if (m_isManaged == managed)'
  '';

  strictDeps = true;
@@ -134,7 +125,9 @@ stdenv.mkDerivation (finalAttrs: {
  meta = {
    description = "Ayatana indiator exporting the network settings menu through D-Bus";
    homepage = "https://gitlab.com/ubports/development/core/lomiri-indicator-network";
    changelog = "https://gitlab.com/ubports/development/core/lomiri-indicator-network/-/blob/${finalAttrs.version}/ChangeLog";
    changelog = "https://gitlab.com/ubports/development/core/lomiri-indicator-network/-/blob/${
      if (!builtins.isNull finalAttrs.src.tag) then finalAttrs.src.tag else finalAttrs.src.rev
    }/ChangeLog";
    license = lib.licenses.gpl3Only;
    teams = [ lib.teams.lomiri ];
    platforms = lib.platforms.linux;