Add tests for UnIndexedFileIterator, which cover basic cases.
Right now it fails, see D21427: Always skip trailing slashes in FilderedDirIterator (one of the folders has trailing slash, while it should not)
Details
Details
- Reviewers
bruns - Group Reviewers
Frameworks Baloo
- Apply also patch from D21427: Always skip trailing slashes in FilderedDirIterator.
- ctest
Diff Detail
Diff Detail
- Repository
- R293 Baloo
- Branch
- unindexed-renamed
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 12910 Build 12928: arc lint + arc unit
Comment Actions
The unit test lacks structure, please think about:
- how you can split this into separate test cases (basic functionality, renaming, ...)
- how to use a data driven approach for the test, i.e. splitting the basic test into one entry per file
| autotests/unit/file/unindexedfileiteratortest.cpp | ||
|---|---|---|
| 126–146 | Bad variable name, these are files and dirs. | |
| 130 | bad name, use something like excluded. | |
| 166 | you don't actually have to determine the mimetype here, just use "text/plain" | |
| src/file/unindexedfileiterator.cpp | ||
| 126 ↗ | (On Diff #58934) | This whole block belongs into a separate block, executed if (m_cTimeChanged). |
Comment Actions
Split test to three separate test functions, which cover different test cases.
Renamed folders to something more meaningful; put all the names as static
consts in the very beginning, for further reuse inside test functions.
Removed useless mimetype detection and use "text/plain" explicitly
| src/file/unindexedfileiterator.cpp | ||
|---|---|---|
| 126 ↗ | (On Diff #58934) | Did you mean m_mTimeChanged? |
| autotests/unit/file/unindexedfileiteratortest.cpp | ||
|---|---|---|
| 100 | Make this plain members, not pointers. | |
| src/file/unindexedfileiterator.cpp | ||
| 122 ↗ | (On Diff #59071) | thats wrong: $:/tmp> touch dir $:/tmp> stat dir File: dir Size: 0 Blocks: 0 IO Block: 4096 regular empty file Device: 3fh/63d Inode: 1892961 Links: 1 Access: (0644/-rw-r--r--) Uid: ( 1000/ sbruens) Gid: ( 100/ users) Access: 2019-06-03 19:21:57.764626372 +0200 Modify: 2019-06-03 19:21:57.764626372 +0200 Change: 2019-06-03 19:21:57.764626372 +0200 Birth: 2019-06-03 19:21:57.764626372 +0200 $:/tmp> mv dir dir_renamed $:/tmp> stat dir_renamed File: dir_renamed Size: 0 Blocks: 0 IO Block: 4096 regular empty file Device: 3fh/63d Inode: 1892961 Links: 1 Access: (0644/-rw-r--r--) Uid: ( 1000/ sbruens) Gid: ( 100/ users) Access: 2019-06-03 19:21:57.764626372 +0200 Modify: 2019-06-03 19:21:57.764626372 +0200 Change: 2019-06-03 19:22:07.096601727 +0200 Birth: 2019-06-03 19:21:57.764626372 +0200 |
| 130 ↗ | (On Diff #59071) | The comment does not match - the mtime changes when something in the directory is modified, when the directory is renamed, the ctime changes. |
| 142 ↗ | (On Diff #59071) | unrelated whitespace change |
| 126 ↗ | (On Diff #58934) | obviously not ... |
| autotests/unit/file/unindexedfileiteratortest.cpp | ||
|---|---|---|
| 100 | I wanted both DB and directory tree to be recreated from scratch for each test, since tests modify DB or rename folders, and thus can affect each other. That's why I made them to be pointers. | |
| src/file/unindexedfileiterator.cpp | ||
| 118 ↗ | (On Diff #59071) | I wanted to note that this line is messed up a bit: either fileMTime should be renamed to fileCTime, or even removed, just like three lines above. Since that's just some cosmetic change, I can change it in this patch as well. Or should I make a new one? |
| 122 ↗ | (On Diff #59071) | Right, sorry, I've mixed them up. Damn russian locale. |
| 130 ↗ | (On Diff #59071) | The comment is not mine - but that's precisely what it says. We don't want to reindex a directory every time something inside it modifies. |
Comment Actions
Rebase on master.
Since we now reindex folder always when cTime changes, revert nameChanged() routine
Cover the case when we feed UnIndexedFileIterator with path that ends with /