> 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");
> > }
> > }
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.
- Frank
-----------------------------------------------------------
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
>
>
