D11034: CoreApplicationSettingsTest

Himanshu Vishwakarma noreply at phabricator.kde.org
Sun Mar 11 11:49:45 UTC 2018


himanshuvishwakarma added inline comments.

INLINE COMMENTS

> jjazeix wrote in ApplicationSettingsTest.cpp:41
> 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)

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.

> jjazeix wrote in ApplicationSettingsTest.cpp:50
> 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

Okay, next time this will not happen and now NO need of this.

> jjazeix wrote in ApplicationSettingsTest.cpp:62
> 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.

here also, I remember it for the next time.

> jjazeix wrote in ApplicationSettingsTest.cpp:143
> variable names should start with a lowercase letter

here also, I remember it for the next time.

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/1d34d397/attachment-0001.html>


More information about the kde-edu mailing list