This is the first unit test of the GCompris. The unit test is testing the main class ActivityInfo in the core which is testing the most of the function of the class.
Details
Diff Detail
- Repository
- R2 GCompris
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Thank you for your contribution, it's a really nice patch (most of the remarks are minor ones).
c++ files are compiled twice (one for the lib, one for the executable). I remembered I had issues (crashes on android) when I tried to link the executable to the library (https://github.com/gcompris/GCompris-qt/blob/networkClient/src/core/CMakeLists.txt#L66 on this branch).
Do you think it can be applied to activities too (as the code is mostly qml and js, it may be more difficult to unit tests them, making automatic tests more viable and interesting to avoid regressions)?
CMakeLists.txt | ||
---|---|---|
462 | if you use automoc, can you remove the qt5_wrap_cpp in the src/core folder and check if it still works? | |
466 | same here, we used include_directories() in the src/core, it may not be needed anymore with this. | |
src/core/CMakeLists.txt | ||
80 | I would name it "gcompris_core" instead of "GCompris" and I would not add "gcompris_RES" to it as they are the resource files and so not linked to the library itself but the executable | |
tests/core/CMakeLists.txt | ||
4 | it would be preferable to not include the folders and use #include "src/core/ActivityInfo.h" instead (so we know where to look for when we look for the file). Also, you probably don't need the resource include | |
15 | it would be better to prefix it with Core instead of GCompris. Basically, all the tests will be for GCompris so no need to add it but we may have different modules (activities, core...). | |
tests/core/activityinfotest.cpp | ||
1 ↗ | (On Diff #28340) | missing licence header |
2 ↗ | (On Diff #28340) | filename should be ActivityInfoTest.cpp |
6 ↗ | (On Diff #28340) | why do you need this? |
41 ↗ | (On Diff #28340) | do you need a pointer here? |
73 ↗ | (On Diff #28340) | I think you can create a macro that will can simplify the tests: #define TEST_ATTRIBUTE(attributeName, accessorName, attributeType) \ { \ QFETCH(attributeType, attributeName); \ QSignalSpy spy(activityinfo, &ActivityInfo::attributeName ## Changed); \ QVERIFY(spy.count() == 0); \ activityinfo->set ## accessorName(attributeName); \ QVERIFY(spy.count() == 1); \ QCOMPARE(activityinfo->##accessorName(), attributeName); \ } and be used for all attributes? |
CMakeLists.txt | ||
---|---|---|
462 | During the writing the test, I don't want to touch the core part of the application. Today I tried to remove the qt5_warp_cpp but it makes complex to handle both the test and application. Maybe I am doing wrong way also. I will try more to make compiled the c++ code once. | |
tests/core/activityinfotest.cpp | ||
6 ↗ | (On Diff #28340) | I just added by mistake |
41 ↗ | (On Diff #28340) | No, Really no needed of the pointer. It is in my practice. Now pointer is removed |
There are some issues when compiling for android (missing libs). I'm fixing them and I'll commit