Make sure that PlacesItemModel is covered by unit test
Details
Diff Detail
- Repository
- R318 Dolphin
- Lint
Lint Skipped - Unit
Unit Tests Skipped
src/tests/placesitemmodeltest.cpp | ||
---|---|---|
114 ↗ | (On Diff #22486) | nitpick: space after for |
115–116 ↗ | (On Diff #22486) | nitpick: missing braces |
157 ↗ | (On Diff #22486) | nullptr |
170 ↗ | (On Diff #22486) | Wrap it in a QVERIFY |
203 ↗ | (On Diff #22486) | Please call it systemRoot. |
217 ↗ | (On Diff #22486) | const |
220–227 ↗ | (On Diff #22486) | .at() instead of [] ? |
270 ↗ | (On Diff #22486) | QUrl::fromLocalFile ? |
274 ↗ | (On Diff #22486) | Please call it ejectAction |
277 ↗ | (On Diff #22486) | Please call it action or teardownAction |
279 ↗ | (On Diff #22486) | What if the action text is translated? |
335 ↗ | (On Diff #22486) | Please call it properties or viewProperties |
400 ↗ | (On Diff #22486) | space after for |
Fixed code style
src/tests/placesitemmodeltest.cpp | ||
---|---|---|
279 ↗ | (On Diff #22486) | Since to safety check for the action label we will need to setup the translate system, I choose to remove this check. Since in the line above we already check if the action exists. |
lgtm, some minor nits only from my side
src/tests/placesitemmodeltest.cpp | ||
---|---|---|
44 | why not QDir rootPath with toNativeSeparators? | |
151 | can you instead use a signal spy and wait for some signal? or use QTRY_COMPARE below? | |
478 | extract this into a helper function to simplify the code, it's repeated in most test functions |
src/tests/placesitemmodeltest.cpp | ||
---|---|---|
44 | This is based on KIO unit test: https://github.com/KDE/kio/blob/master/autotests/kfileplacesmodeltest.cpp#L34 Not sure the reason for that. But I can change it to use QDir as requested if you think that will not cause any problem. | |
151 | I tried all models signals, and none is fired. |
I'm getting this failure when running the test:
********* Start testing of PlacesItemModelTest ********* Config: Using QtTest library 5.10.0, Qt 5.10.0 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 7.2.0) WARNING: PlacesItemModelTest::initTestCase() testdata fakecomputer.xml could not be located! Loc: [../src/tests/placesitemmodeltest.cpp(177)] FAIL! : PlacesItemModelTest::initTestCase() '!fakeHw.isEmpty()' returned FALSE. () Loc: [../src/tests/placesitemmodeltest.cpp(178)] PASS : PlacesItemModelTest::cleanupTestCase() Totals: 1 passed, 1 failed, 0 skipped, 0 blacklisted, 0ms
Am I missing something?
src/tests/placesitemmodeltest.cpp | ||
---|---|---|
184 ↗ | (On Diff #22486) | QFileInfo::exists would be faster |
src/tests/placesitemmodeltest.cpp | ||
---|---|---|
178 ↗ | (On Diff #22486) | Can you please move the xml file into a data/ subfolder and use QFINDTESTDATA("data/fakecomputer.xml") here? Other than that, looks good to me now. |