> On July 28, 2011, 10:26 a.m., Aurélien Gâteau wrote: > > I don't think it is good for unit-tests to test different things depending > > 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 <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::self(), > > * and then we get another process (kglobalsettingsclient) to call > > emitChange(), > > * 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 > > session. If > > + // we are not then KDE applications follow the platform palette and > > font > > + // settings. > > + setenv("KDE_FULL_SESSION", "1", 1); > > + > > QDBusConnectionInterface *bus = 0; > > if (!QDBusConnection::sessionBus().isConnected() || !(bus = > > QDBusConnection::sessionBus().interface())) { > > QFAIL("Session bus not found"); > > } > > } > > Frank Reininghaus wrote: > I fully agree that the things that are tested should better not depend on > environment variables, and I can confirm that the test passes here with your > patch applied. I hadn't tried this myself earlier because I had assumed that > the 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 > 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 'run outside KDE session' situation? - Aurélien ----------------------------------------------------------- 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élien Gâteau and David Faure. > > > Summary > ------- > > Since commit b064749a754ec358170ecb7f19828e4216f6e965, KDE palette 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 full KDE session, > see > > http://my.cdash.org/testSummary.php?project=16&name=kdeui-kglobalsettingstest&date=2011-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 apps > 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 > regular user's KDE 4.6 session). > > > Thanks, > > Frank > >