nmc/add new unit test files #5867
Conversation
|
AppImage file: nextcloud-PR-5867-2f4a680ba113f2618a01fd27d1655184cd968fd6-x86_64.AppImage |
There was a problem hiding this comment.
please see my comments about #define private public
during the first review, I had overlook that you are testing some qml components
this PR lacking the content of the tests makes it a bit difficult to review those tests but I am really surprised by the way you are writing qml tests ?
I would really recommend following the approach of having a generic qml tests runner and then writing qml test code ?
a bit like what they explain in their doc
| #define private public | ||
| #include "folderstatusdelegate.h" | ||
| #undef private |
There was a problem hiding this comment.
this "trick" will work because we are using a static library
if we would have been using a shared library (DLL on windows) I think that would fail because the symbol mangling is different for private/public class members (see https://en.wikiversity.org/wiki/Visual_C%2B%2B_name_mangling#Complex_Type_(union,_struct,_class,_coclass,_cointerface) and https://www.qt.io/blog/2009/08/12/some-thoughts-on-binary-compatibility)
I would not block this PR only based on that but
- we would be unable to keep using that if our code was moved to a shared library (DLL)
- I am not convinced of automated tests using private interfaces
- That creates heavy coupling of the tests to the current internal implementation
why not "just" make your test a friend of the class you are testing ? That would also be more in line with the current state of the automated tests of the desktop client ?
There was a problem hiding this comment.
Yes, friend class is very simple and easy solution, I avoided this because of only data/member privacy concern.
but if this is not major concern then I will apply the same.
| #define private public | ||
| #include "accountstate.h" | ||
| #include "syncresult.h" | ||
| #include "folder.h" | ||
| #include "theme.h" | ||
| #undef private |
| #include "theme.h" | ||
| #include "folderman.h" | ||
| #include "configfile.h" | ||
| #define private public |
|
DCO check is failing |
| @@ -153,6 +154,7 @@ private slots: | |||
| bool _dirty = false; // If the selective sync checkboxes were changed | |||
|
|
|||
| bool _isSyncRunningForAwhile = false; | |||
| friend class:: TestFolderStatusModel;// for Unit Test | |||
There was a problem hiding this comment.
| friend class:: TestFolderStatusModel;// for Unit Test | |
| friend class ::TestFolderStatusModel; // for Unit Test |
Added new Unit Test files(which are not present in Nextcloud repo) with template, dependent header files and some member access dependency.
Later we will add testcase for NMC changes & nextcloud can also add test cases.