D11034: CoreApplicationSettingsTest

[Johnny Jazeix]

jjazeix added inline comments.

INLINE COMMENTS

> himanshuvishwakarma wrote in ApplicationSettingsTest.cpp:41
> Yaa, I tried both methods but both of them compelled to change in class ApplicationSettings. which we have to test. So I find a nice approach by this our default configuration is not going to change.
> 
> In this method we are first saving the default data(which is already present in the config file ), then test the unit test with our data and again reset the default data.
> 
> In this approach, we don't need to make the dummy class for the testing and not to change in the ApplicationSettings class.
> 
> now everything is working fine.

What happens if for some reason the test crashes while there is a change done?
Also, you are still adding dummy things in the real configuration that has nothing to do with the real application.
You should not touch real application data when doing tests.

> ApplicationSettingsTest.cpp:44
> +private slots:
> +	void ApplicationSettingsInitializationTest();
> +	void ApplicationSettingsTest();

indentation should be 4 spaces on c++ files.

> CMakeLists.txt:15
> +
> +add_executable(CoreApplicationSettingsTest ApplicationSettingsTest.cpp)
> +target_link_libraries(CoreApplicationSettingsTest ${CORE_TEST_LIBRARIES})

can you do a macro or function to avoid duplicating these 3 lines everytime?

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/20180311/5d1386c1/attachment.html>