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 | ||
---|---|---|
115 ↗ | (On Diff #22481) | nitpick: space after for |
116–117 ↗ | (On Diff #22481) | nitpick: missing braces |
158 ↗ | (On Diff #22481) | nullptr |
171 ↗ | (On Diff #22481) | Wrap it in a QVERIFY |
204 ↗ | (On Diff #22481) | Please call it systemRoot. |
218 ↗ | (On Diff #22481) | const |
221–228 ↗ | (On Diff #22481) | .at() instead of [] ? |
271 ↗ | (On Diff #22481) | QUrl::fromLocalFile ? |
275 ↗ | (On Diff #22481) | Please call it ejectAction |
278 ↗ | (On Diff #22481) | Please call it action or teardownAction |
280 ↗ | (On Diff #22481) | What if the action text is translated? |
336 ↗ | (On Diff #22481) | Please call it properties or viewProperties |
401 ↗ | (On Diff #22481) | space after for |
Fixed code style
src/tests/placesitemmodeltest.cpp | ||
---|---|---|
280 ↗ | (On Diff #22481) | 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 | ||
---|---|---|
45 ↗ | (On Diff #22481) | why not QDir rootPath with toNativeSeparators? |
152 ↗ | (On Diff #22481) | can you instead use a signal spy and wait for some signal? or use QTRY_COMPARE below? |
479 ↗ | (On Diff #22481) | extract this into a helper function to simplify the code, it's repeated in most test functions |
src/tests/placesitemmodeltest.cpp | ||
---|---|---|
45 ↗ | (On Diff #22481) | 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. |
152 ↗ | (On Diff #22481) | 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 #22481) | QFileInfo::exists would be faster |
src/tests/placesitemmodeltest.cpp | ||
---|---|---|
178 ↗ | (On Diff #22481) | 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. |