This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102098/ |
On July 28th, 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"); } }On July 28th, 2011, 11:33 a.m., 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
On July 27th, 2011, 4:31 p.m., Frank Reininghaus wrote:
Review request for kdelibs, Aurélien Gâteau and David Faure.
By Frank Reininghaus.
Updated July 27, 2011, 4:31 p.m. Description
Testing
Diffs
|