D11034: CoreApplicationSettingsTest

[Johnny Jazeix]

jjazeix added inline comments.

INLINE COMMENTS

> himanshuvishwakarma wrote in ApplicationSettingsTest.cpp:41
> try to make a separate config file for the test which does not affect the application settings. But not get success. Any help or suggestion ??

I see 2 potential solutions:
either inheritance and protected
or 
friend keyword.

I let you investigate a bit more to look for them and choose :).
(I don't like the 3rd solution which would be "#define private public" in the test file)

> ApplicationSettingsTest.cpp:50
> +private:
> +	QSettings m_config;
> +};

you don't have to redefine a new attribute with the same name, it won't do what you expect and it does not correspond to the one in the parent class

> ApplicationSettingsTest.cpp:62
> +private:
> +	DummyApplicationSettings* dummyApplicationSettings =  new DummyApplicationSettings();
> +};

this one is never freed so it leaks, don't use pointer when you don't need them :)
And don't use the same object shared between different tests, there might be side effects.

> ApplicationSettingsTest.cpp:143
> +	// Creating a dummyActivity
> +	QString DummyActivity = QStringLiteral("DummyActivity");
> +

variable names should start with a lowercase letter

REPOSITORY
  R2 GCompris

REVISION DETAIL
  https://phabricator.kde.org/D11034

To: himanshuvishwakarma, jjazeix, dmadaan, rudranilbasu, timotheegiet, #gcompris
Cc: #kde_edu, #gcompris, harrymecwan, ganeshredcobra, nityanandkumar, echarruau, rahulyadav, narvaez, scagarwal, apol, timotheegiet, hkaelberer, jjazeix, bcoudoin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20180310/2f1f9004/attachment.html>