--===============9192025487294155983== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On July 28, 2011, 10:26 a.m., Aur=C3=A9lien G=C3=A2teau wrote: > > I don't think it is good for unit-tests to test different things depend= ing on environment variables ( > > @David, what is your opinion on the subject?) > > = > > I was actually about to commit a simpler fix: > > = > > @@ -26,11 +26,11 @@ QTEST_KDEMAIN( KGlobalSettingsTest, GUI ) > > #include > > #include > > #include > > #include > > #include > > - > > +#include > > /** > > * The strategy of this test is: > > * We install QSignalSpy instances on many signals from KGlobalSetting= s::self(), > > * and then we get another process (kglobalsettingsclient) to call emi= tChange(), > > * and we check that the corresponding signals are emitted, i.e. that = our process > > @@ -39,10 +39,15 @@ QTEST_KDEMAIN( KGlobalSettingsTest, GUI ) > > * As a nice side-effect we automatically test a bit of KProcess as we= ll :) > > */ > > = > > void KGlobalSettingsTest::initTestCase() > > { > > + // Some signals are only emitted when we are running a full KDE se= ssion. If > > + // we are not then KDE applications follow the platform palette an= d font > > + // settings. > > + setenv("KDE_FULL_SESSION", "1", 1); > > + > > QDBusConnectionInterface *bus =3D 0; > > if (!QDBusConnection::sessionBus().isConnected() || !(bus =3D QDBu= sConnection::sessionBus().interface())) { > > QFAIL("Session bus not found"); > > } > > } > = > Frank Reininghaus wrote: > I fully agree that the things that are tested should better not depen= d on environment variables, and I can confirm that the test passes here wit= h your patch applied. I hadn't tried this myself earlier because I had assu= med that the variable has to be set *before* the test executable is run, bu= t that's obviously wrong ;-) > = > Maybe it would be even better to always test both things (i.e., that = signals are emitted when KDE_FULL_SESSION is set and that they aren't when = it's not set) if we find a good way to do it? That way, we would also test = that the things that your recent commit changed work as they should. I agree testing both would be even nicer. Maybe you can mix your patch with= mine to do it, calling setenv("KDE_FULL_SESSION", "1", 1) to fake the "run= in KDE session" situation and unsetenv("KDE_FULL_SESSION") to fake the 'ru= n outside KDE session' situation? - Aur=C3=A9lien ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102098/#review5166 ----------------------------------------------------------- On July 27, 2011, 4:31 p.m., Frank Reininghaus wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102098/ > ----------------------------------------------------------- > = > (Updated July 27, 2011, 4:31 p.m.) > = > = > Review request for kdelibs, Aur=C3=A9lien G=C3=A2teau and David Faure. > = > = > Summary > ------- > = > Since commit b064749a754ec358170ecb7f19828e4216f6e965, KDE palette and fo= nt settings are only used when running KDE apps in a full KDE session. This= makes KGlobalSettingsTest fail if the test is not run in a full KDE sessio= n, see > = > http://my.cdash.org/testSummary.php?project=3D16&name=3Dkdeui-kglobalsett= ingstest&date=3D2011-07-27 > = > This commit changes KGlobalSettings' unit test to reflect that change. My= first idea was to change the unit test such that it verifies the expected = behaviour for both situations, i.e., apps run in a full KDE session and app= s run in some other kind of session, but I could not figure out a way to do= this without changing the KDE_FULL_SESSION environment variable before the= unit test executable is run. > = > In the case that the signal is not expected, I reduced the kWaitForSignal= timeout to prevent wasting too much time each time the test is run. > = > = > Diffs > ----- > = > kdeui/tests/kglobalsettingstest.h 69ed5bf = > kdeui/tests/kglobalsettingstest.cpp 464825d = > = > Diff: http://git.reviewboard.kde.org/r/102098/diff > = > = > Testing > ------- > = > The test passes here (run by my kde-devel user in a Konsole inside the re= gular user's KDE 4.6 session). > = > = > Thanks, > = > Frank > = > --===============9192025487294155983== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/102098/

On July 28th, 2011, 10:26 a.m., Aur=C3=A9li= en G=C3=A2teau wrote:

I don'=
;t think it is good for unit-tests to test different things depending on en=
vironment variables (
@David, what is your opinion on the subject?)

I was actually about to commit a simpler fix:

@@ -26,11 +26,11 @@ QTEST_KDEMAIN( KGlobalSettingsTest, GUI )
 #include <kglobalsettings.h>
 #include <kdebug.h>
 #include <kprocess.h>
 #include <QtCore/QEventLoop>
 #include <QtDBus/QtDBus>
-
+#include <stdlib.h>
 /**
  * The strategy of this test is:
  * We install QSignalSpy instances on many signals from KGlobalSettings::s=
elf(),
  * and then we get another process (kglobalsettingsclient) to call emitCha=
nge(),
  * and we check that the corresponding signals are emitted, i.e. that our =
process
@@ -39,10 +39,15 @@ QTEST_KDEMAIN( KGlobalSettingsTest, GUI )
  * As a nice side-effect we automatically test a bit of KProcess as well :)
  */
 =

 void KGlobalSettingsTest::initTestCase()
 {
+    // Some signals are only emitted when we are running a full KDE sessio=
n. If
+    // we are not then KDE applications follow the platform palette and fo=
nt
+    // settings.
+    setenv("KDE_FULL_SESSION", "1", 1);
+
     QDBusConnectionInterface *bus =3D 0;
     if (!QDBusConnection::sessionBus().isConnected() || !(bus =3D QDBusCon=
nection::sessionBus().interface())) {
         QFAIL("Session bus not found");
     }
 }

On July 28th, 2011, 11:33 a.m., Frank Reininghaus wrote:

I fully a=
gree that the things that are tested should better not depend on environmen=
t variables, and I can confirm that the test passes here with your patch ap=
plied. I hadn't tried this myself earlier because I had assumed that th=
e variable has to be set *before* the test executable is run, but that'=
s obviously wrong ;-)

Maybe it would be even better to always test both things (i.e., that signal=
s are emitted when KDE_FULL_SESSION is set and that they aren't when it=
's not set) if we find a good way to do it? That way, we would also tes=
t that the things that your recent commit changed work as they should.
I agree tes=
ting both would be even nicer. Maybe you can mix your patch with mine to do=
 it, calling setenv("KDE_FULL_SESSION", "1", 1) to fake=
 the "run in KDE session" situation and unsetenv("KDE_FULL_S=
ESSION") to fake the 'run outside KDE session' situation?

- Aur=C3=A9lien


On July 27th, 2011, 4:31 p.m., Frank Reininghaus wrote:

Review request for kdelibs, Aur=C3=A9lien G=C3=A2teau and David Faure.=
By Frank Reininghaus.

Updated July 27, 2011, 4:31 p.m.

Descripti= on 

Since commit b064749a754ec358170ecb7f19828e4216f6e965, KDE p=
alette and font settings are only used when running KDE apps in a full KDE =
session. This makes KGlobalSettingsTest fail if the test is not run in a fu=
ll KDE session, see

http://my.cdash.org/testSummary.php?project=3D16&name=3Dkdeui-kglobalse=
ttingstest&date=3D2011-07-27

This commit changes KGlobalSettings' unit test to reflect that change. =
My first idea was to change the unit test such that it verifies the expecte=
d behaviour for both situations, i.e., apps run in a full KDE session and a=
pps run in some other kind of session, but I could not figure out a way to =
do this without changing the KDE_FULL_SESSION environment variable before t=
he unit test executable is run.

In the case that the signal is not expected, I reduced the kWaitForSignal t=
imeout to prevent wasting too much time each time the test is run.

Testing <= /h1> 
The test passes here (run by my kde-devel user in a Konsole =
inside the regular user's KDE 4.6 session).

Diffs=

  • kdeui/tests/kglobalsettingstest.h (69ed5bf= )
  • kdeui/tests/kglobalsettingstest.cpp (46482= 5d)

View Diff
--===============9192025487294155983==--