Changeset View
Standalone View
autotests/jobtest.cpp
Show First 20 Lines • Show All 1374 Lines • ▼ Show 20 Line(s) | |||||
1375 | } | 1375 | } | ||
1376 | 1376 | | |||
1377 | void JobTest::stat() | 1377 | void JobTest::stat() | ||
1378 | { | 1378 | { | ||
1379 | #if 1 | 1379 | #if 1 | ||
1380 | const QString filePath = homeTmpDir() + "fileFromHome"; | 1380 | const QString filePath = homeTmpDir() + "fileFromHome"; | ||
1381 | createTestFile(filePath); | 1381 | createTestFile(filePath); | ||
1382 | const QUrl url(QUrl::fromLocalFile(filePath)); | 1382 | const QUrl url(QUrl::fromLocalFile(filePath)); | ||
1383 | KIO::StatJob *job = KIO::stat(url, KIO::HideProgressInfo); | 1383 | KIO::StatJob *job = KIO::stat(url, KIO::HideProgressInfo); | ||
meven: Please add a separate test dedicated for Inode like `statWithInode()` like `statSymlink` does… | |||||
ahmadsamir: Righto. Done. | |||||
1384 | QVERIFY(job); | 1384 | QVERIFY(job); | ||
1385 | QVERIFY2(job->exec(), qPrintable(job->errorString())); | 1385 | QVERIFY2(job->exec(), qPrintable(job->errorString())); | ||
1386 | // TODO set setSide | 1386 | // TODO set setSide | ||
1387 | const KIO::UDSEntry &entry = job->statResult(); | 1387 | const KIO::UDSEntry &entry = job->statResult(); | ||
1388 | 1388 | | |||
1389 | // we only get filename, access, type, size, uid, gid, btime, mtime, atime | 1389 | // we only get filename, access, type, size, uid, gid, btime, mtime, atime | ||
1390 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_NAME)); | 1390 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_NAME)); | ||
1391 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_ACCESS)); | 1391 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_ACCESS)); | ||
1392 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_SIZE)); | 1392 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_SIZE)); | ||
1393 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_FILE_TYPE)); | 1393 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_FILE_TYPE)); | ||
1394 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_USER)); | 1394 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_USER)); | ||
1395 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_GROUP)); | 1395 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_GROUP)); | ||
1396 | //QVERIFY(entry.contains(KIO::UDSEntry::UDS_CREATION_TIME)); // only true if st_birthtime or statx is used | 1396 | //QVERIFY(entry.contains(KIO::UDSEntry::UDS_CREATION_TIME)); // only true if st_birthtime or statx is used | ||
1397 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_MODIFICATION_TIME)); | 1397 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_MODIFICATION_TIME)); | ||
1398 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_ACCESS_TIME)); | 1398 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_ACCESS_TIME)); | ||
1399 | QCOMPARE(entry.count(), 8 + (entry.contains(KIO::UDSEntry::UDS_CREATION_TIME) ? 1 : 0)); | 1399 | QCOMPARE(entry.count(), 8 + (entry.contains(KIO::UDSEntry::UDS_CREATION_TIME) ? 1 : 0)); | ||
1400 | 1400 | | |||
UDS_CREATION_TIME is statx specific only on Linux meven: UDS_CREATION_TIME is statx specific only on Linux
Just make it clear in the comment. | |||||
In the new unit test, I think using HAVE_STATX is self-explanatory (I'll leave JobTest::stat() alone in this diff to keep the commit atomic). ahmadsamir: In the new unit test, I think using HAVE_STATX is self-explanatory (I'll leave JobTest::stat()… | |||||
1401 | QVERIFY(!entry.isDir()); | 1401 | QVERIFY(!entry.isDir()); | ||
1402 | QVERIFY(!entry.isLink()); | 1402 | QVERIFY(!entry.isLink()); | ||
KIO::UDSEntry::UDS_DEVICE_ID and KIO::UDSEntry::UDS_INODE are not statx specific (it is supported for all UNIX systems) meven: KIO::UDSEntry::UDS_DEVICE_ID and KIO::UDSEntry::UDS_INODE are not statx specific (it is… | |||||
1403 | QCOMPARE(entry.stringValue(KIO::UDSEntry::UDS_NAME), QStringLiteral("fileFromHome")); | 1403 | QCOMPARE(entry.stringValue(KIO::UDSEntry::UDS_NAME), QStringLiteral("fileFromHome")); | ||
1404 | 1404 | | |||
1405 | // Compare what we get via kio_file and what we get when KFileItem stat()s directly | 1405 | // Compare what we get via kio_file and what we get when KFileItem stat()s directly | ||
1406 | const KFileItem kioItem(entry, url); | 1406 | const KFileItem kioItem(entry, url); | ||
1407 | const KFileItem fileItem(url); | 1407 | const KFileItem fileItem(url); | ||
1408 | QCOMPARE(kioItem.name(), fileItem.name()); | 1408 | QCOMPARE(kioItem.name(), fileItem.name()); | ||
1409 | QCOMPARE(kioItem.url(), fileItem.url()); | 1409 | QCOMPARE(kioItem.url(), fileItem.url()); | ||
1410 | QCOMPARE(kioItem.size(), fileItem.size()); | 1410 | QCOMPARE(kioItem.size(), fileItem.size()); | ||
▲ Show 20 Lines • Show All 89 Lines • ▼ Show 20 Line(s) | 1470 | { | |||
1500 | QCOMPARE(kioItem.user(), ""); | 1500 | QCOMPARE(kioItem.user(), ""); | ||
1501 | QCOMPARE(kioItem.group(), ""); | 1501 | QCOMPARE(kioItem.group(), ""); | ||
1502 | QCOMPARE(kioItem.mimetype(), "application/octet-stream"); | 1502 | QCOMPARE(kioItem.mimetype(), "application/octet-stream"); | ||
1503 | QCOMPARE(kioItem.permissions(), 438); | 1503 | QCOMPARE(kioItem.permissions(), 438); | ||
1504 | QCOMPARE(kioItem.time(KFileItem::ModificationTime), QDateTime()); | 1504 | QCOMPARE(kioItem.time(KFileItem::ModificationTime), QDateTime()); | ||
1505 | QCOMPARE(kioItem.time(KFileItem::AccessTime), QDateTime()); | 1505 | QCOMPARE(kioItem.time(KFileItem::AccessTime), QDateTime()); | ||
1506 | } | 1506 | } | ||
1507 | 1507 | | |||
1508 | void JobTest::statUniqueDeviceIDs() | ||||
1509 | { | ||||
1510 | const QString filePath = homeTmpDir() + "fileFromHome"; | ||||
bruns: This should only apply to the `QVERIFY(device != otherDevice);` | |||||
Why? The goal is to test if the UDS_DEVICE_ID is unique for individual partitions, which won't work unless there are two separate partitions. ahmadsamir: Why? The goal is to test if the UDS_DEVICE_ID is unique for individual partitions, which won't… | |||||
The first goal should be to test if it works at all, i.e. if there is an UDS_DEVICE_ID/INODE at all. bruns: The first goal should be to test if it works at all, i.e. if there is an UDS_DEVICE_ID/INODE at… | |||||
ahmadsamir: OK, that makes sense. Done. | |||||
bruns: nitpick - the test name now again is wrong ;-)
| |||||
1511 | createTestFile(filePath); | ||||
1512 | const QUrl url(QUrl::fromLocalFile(filePath)); | ||||
1513 | KIO::StatJob *job = KIO::statDetails(url, KIO::StatJob::SourceSide, KIO::StatInode); | ||||
1514 | QVERIFY(job); | ||||
1515 | QVERIFY2(job->exec(), qPrintable(job->errorString())); | ||||
1516 | | ||||
meven: Can remove | |||||
1517 | const KIO::UDSEntry &entry = job->statResult(); | ||||
1518 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_DEVICE_ID)); | ||||
1519 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_INODE)); | ||||
1520 | QCOMPARE(entry.count(), 2); | ||||
1521 | | ||||
1522 | const QString path = otherTmpDir() + "otherFile"; | ||||
1523 | createTestFile(path); | ||||
1524 | const QUrl otherUrl(QUrl::fromLocalFile(path)); | ||||
meven: add `QCOMPARE(entry.count(), 2);` | |||||
bruns: Its 'statDetails**With**Inode' not 'statDetailsInode**Only**' | |||||
bruns: i.e. the test should be renamed now. | |||||
1525 | KIO::StatJob *otherJob = KIO::statDetails(otherUrl, KIO::StatJob::SourceSide, KIO::StatInode); | ||||
1526 | QVERIFY(otherJob); | ||||
1527 | QVERIFY2(otherJob->exec(), qPrintable(otherJob->errorString())); | ||||
1528 | | ||||
1529 | const KIO::UDSEntry otherEntry = otherJob->statResult(); | ||||
1530 | QVERIFY(otherEntry.contains(KIO::UDSEntry::UDS_DEVICE_ID)); | ||||
If you want to avoid doing this you can remove fields asked from statDetails KIO::StatBasic | KIO::StatUser | KIO::StatInode for instance. meven: If you want to avoid doing this you can remove fields asked from statDetails `KIO::StatBasic |… | |||||
Good point, I slimmed it down to just KIO::StatInode. (Maybe that's too thin?). ahmadsamir: Good point, I slimmed it down to just KIO::StatInode. (Maybe that's too thin?). | |||||
1531 | QVERIFY(otherEntry.contains(KIO::UDSEntry::UDS_INODE)); | ||||
1532 | QCOMPARE(otherEntry.count(), 2); | ||||
1533 | | ||||
1534 | const int device = entry.numberValue(KIO::UDSEntry::UDS_DEVICE_ID); | ||||
1535 | const int otherDevice = otherEntry.numberValue(KIO::UDSEntry::UDS_DEVICE_ID); | ||||
meven: add `QCOMPARE(otherEntry.count(), 2);` | |||||
1536 | | ||||
1537 | // this test doesn't make sense on the CI as it's an LXC container with one partition | ||||
slight rewording // In case the two tmp dirs are on the same partition (e.g. on the CI), the // device IDs should match, otherwise they should be different and then } else { QCOMPARE(device, otherDevice); } bruns: slight rewording
```
// In case the two tmp dirs are on the same partition (e.g. on the CI)… | |||||
1538 | if (otherTmpDirIsOnSamePartition()) { | ||||
1539 | QVERIFY(device != otherDevice); | ||||
1540 | } | ||||
bruns: QCOMPARE(device, otherDevice); | |||||
1541 | } | ||||
1542 | | ||||
1508 | #ifndef Q_OS_WIN | 1543 | #ifndef Q_OS_WIN | ||
1509 | void JobTest::statSymlink() | 1544 | void JobTest::statSymlink() | ||
1510 | { | 1545 | { | ||
1511 | const QString filePath = homeTmpDir() + "fileFromHome"; | 1546 | const QString filePath = homeTmpDir() + "fileFromHome"; | ||
1512 | createTestFile(filePath); | 1547 | createTestFile(filePath); | ||
1513 | const QString symlink = otherTmpDir() + "link"; | 1548 | const QString symlink = otherTmpDir() + "link"; | ||
1514 | QVERIFY(QFile(filePath).link(symlink)); | 1549 | QVERIFY(QFile(filePath).link(symlink)); | ||
1515 | QVERIFY(QFile::exists(symlink)); | 1550 | QVERIFY(QFile::exists(symlink)); | ||
1516 | setTimeStamp(symlink, QDateTime::currentDateTime().addSecs(-20)); // differentiate link time and source file time | 1551 | setTimeStamp(symlink, QDateTime::currentDateTime().addSecs(-20)); // differentiate link time and source file time | ||
1517 | 1552 | | |||
1518 | const QUrl url(QUrl::fromLocalFile(symlink)); | 1553 | const QUrl url(QUrl::fromLocalFile(symlink)); | ||
1519 | KIO::StatJob *job = KIO::statDetails(url, KIO::StatJob::StatSide::SourceSide, | 1554 | KIO::StatJob *job = KIO::statDetails(url, KIO::StatJob::StatSide::SourceSide, | ||
1520 | KIO::StatBasic | KIO::StatResolveSymlink | KIO::StatUser | KIO::StatTime, | 1555 | KIO::StatBasic | KIO::StatResolveSymlink | KIO::StatUser | KIO::StatTime, | ||
1521 | KIO::HideProgressInfo); | 1556 | KIO::HideProgressInfo); | ||
1522 | QVERIFY(job); | 1557 | QVERIFY(job); | ||
1523 | QVERIFY2(job->exec(), qPrintable(job->errorString())); | 1558 | QVERIFY2(job->exec(), qPrintable(job->errorString())); | ||
1524 | // TODO set setSide, setDetails | 1559 | // TODO set setSide, setDetails | ||
1525 | const KIO::UDSEntry &entry = job->statResult(); | 1560 | const KIO::UDSEntry &entry = job->statResult(); | ||
1526 | 1561 | | |||
1527 | // we only get filename, access, type, size, linkdest, uid, gid, btime, mtime, atime | 1562 | // we only get filename, access, type, size, linkdest, uid, gid, btime, mtime, atime | ||
1528 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_NAME)); | 1563 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_NAME)); | ||
1529 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_ACCESS)); | 1564 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_ACCESS)); | ||
1530 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_SIZE)); | 1565 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_SIZE)); | ||
1531 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_FILE_TYPE)); | 1566 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_FILE_TYPE)); | ||
meven: You can remove this now ;) | |||||
1532 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_LINK_DEST)); | 1567 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_LINK_DEST)); | ||
1533 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_USER)); | 1568 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_USER)); | ||
You can have an else with just QCOMPARE(device, otherDevice); and move most of the code inside of if (!otherTmpDirIsOnSamePartition()) outside of the if branch. meven: You can have an else with just ` QCOMPARE(device, otherDevice);` and move most of the code… | |||||
I moved the condition to the top, if we can't find two partitions on the system, bail out early. ahmadsamir: I moved the condition to the top, if we can't find two partitions on the system, bail out early. | |||||
1534 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_GROUP)); | 1569 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_GROUP)); | ||
1535 | //QVERIFY(entry.contains(KIO::UDSEntry::UDS_CREATION_TIME)); // only true if st_birthtime or statx is used | 1570 | //QVERIFY(entry.contains(KIO::UDSEntry::UDS_CREATION_TIME)); // only true if st_birthtime or statx is used | ||
1536 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_MODIFICATION_TIME)); | 1571 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_MODIFICATION_TIME)); | ||
1537 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_ACCESS_TIME)); | 1572 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_ACCESS_TIME)); | ||
1538 | QCOMPARE(entry.count(), 9 + (entry.contains(KIO::UDSEntry::UDS_CREATION_TIME) ? 1 : 0)); | 1573 | QCOMPARE(entry.count(), 9 + (entry.contains(KIO::UDSEntry::UDS_CREATION_TIME) ? 1 : 0)); | ||
1539 | 1574 | | |||
1540 | QVERIFY(!entry.isDir()); | 1575 | QVERIFY(!entry.isDir()); | ||
1541 | QVERIFY(entry.isLink()); | 1576 | QVERIFY(entry.isLink()); | ||
▲ Show 20 Lines • Show All 715 Lines • Show Last 20 Lines |
Please add a separate test dedicated for Inode like statWithInode() like statSymlink does for instance.
Here you change the purpose of this test : default stat behavior with stat(), to default + inode with statDetails.