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::statDetailsWithInode() | ||||
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::StatDefaultDetails | KIO::StatInode); | ||||
1514 | QVERIFY(job); | ||||
1515 | QVERIFY2(job->exec(), qPrintable(job->errorString())); | ||||
1516 | // TODO set setSide | ||||
meven: Can remove | |||||
1517 | const KIO::UDSEntry &entry = job->statResult(); | ||||
1518 | | ||||
1519 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_NAME)); | ||||
1520 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_ACCESS)); | ||||
1521 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_SIZE)); | ||||
1522 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_FILE_TYPE)); | ||||
1523 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_USER)); | ||||
1524 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_GROUP)); | ||||
meven: add `QCOMPARE(entry.count(), 2);` | |||||
bruns: Its 'statDetails**With**Inode' not 'statDetailsInode**Only**' | |||||
bruns: i.e. the test should be renamed now. | |||||
1525 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_MODIFICATION_TIME)); | ||||
1526 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_ACCESS_TIME)); | ||||
1527 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_DEVICE_ID)); | ||||
1528 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_INODE)); | ||||
1529 | #if 1 // should be #ifdef HAVE_STATX | ||||
1530 | QVERIFY(entry.contains(KIO::UDSEntry::UDS_CREATION_TIME)); | ||||
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 | QCOMPARE(entry.count(), 11); | ||||
1532 | #else | ||||
1533 | QCOMPARE(entry.count(), 10); | ||||
1534 | #endif | ||||
1535 | | ||||
meven: add `QCOMPARE(otherEntry.count(), 2);` | |||||
1536 | QVERIFY(!entry.isDir()); | ||||
1537 | QVERIFY(!entry.isLink()); | ||||
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 | QCOMPARE(entry.stringValue(KIO::UDSEntry::UDS_NAME), QStringLiteral("fileFromHome")); | ||||
1539 | | ||||
1540 | // Compare what we get via kio_file and what we get when KFileItem stat()s directly | ||||
bruns: QCOMPARE(device, otherDevice); | |||||
1541 | const KFileItem kioItem(entry, url); | ||||
1542 | const KFileItem fileItem(url); | ||||
1543 | QCOMPARE(kioItem.name(), fileItem.name()); | ||||
1544 | QCOMPARE(kioItem.url(), fileItem.url()); | ||||
1545 | QCOMPARE(kioItem.size(), fileItem.size()); | ||||
1546 | QCOMPARE(kioItem.user(), fileItem.user()); | ||||
1547 | QCOMPARE(kioItem.group(), fileItem.group()); | ||||
1548 | QCOMPARE(kioItem.mimetype(), fileItem.mimetype()); | ||||
1549 | QCOMPARE(kioItem.permissions(), fileItem.permissions()); | ||||
1550 | QCOMPARE(kioItem.time(KFileItem::ModificationTime), fileItem.time(KFileItem::ModificationTime)); | ||||
1551 | QCOMPARE(kioItem.time(KFileItem::AccessTime), fileItem.time(KFileItem::AccessTime)); | ||||
1552 | | ||||
1553 | // this part doesn't make sense on the CI as it's an LXC container with one partition | ||||
1554 | if (!otherTmpDirIsOnSamePartition()) { | ||||
1555 | const QString path = otherTmpDir() + "otherFile"; | ||||
1556 | createTestFile(path); | ||||
1557 | const QUrl otherUrl(QUrl::fromLocalFile(path)); | ||||
1558 | KIO::StatJob *otherJob = KIO::statDetails(otherUrl, KIO::StatJob::SourceSide, | ||||
1559 | KIO::StatDefaultDetails | KIO::StatInode); | ||||
1560 | QVERIFY(otherJob); | ||||
1561 | QVERIFY2(otherJob->exec(), qPrintable(otherJob->errorString())); | ||||
1562 | const KIO::UDSEntry otherEntry = otherJob->statResult(); | ||||
1563 | | ||||
1564 | const int device = entry.numberValue(KIO::UDSEntry::UDS_DEVICE_ID); | ||||
1565 | const int otherDevice = otherEntry.numberValue(KIO::UDSEntry::UDS_DEVICE_ID); | ||||
1566 | qDebug() << "device ID:" << device << "; otherDevice ID:" << otherDevice; | ||||
meven: You can remove this now ;) | |||||
1567 | QVERIFY(device != otherDevice); | ||||
1568 | } | ||||
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. | |||||
1569 | } | ||||
1570 | | ||||
1508 | #ifndef Q_OS_WIN | 1571 | #ifndef Q_OS_WIN | ||
1509 | void JobTest::statSymlink() | 1572 | void JobTest::statSymlink() | ||
1510 | { | 1573 | { | ||
1511 | const QString filePath = homeTmpDir() + "fileFromHome"; | 1574 | const QString filePath = homeTmpDir() + "fileFromHome"; | ||
1512 | createTestFile(filePath); | 1575 | createTestFile(filePath); | ||
1513 | const QString symlink = otherTmpDir() + "link"; | 1576 | const QString symlink = otherTmpDir() + "link"; | ||
1514 | QVERIFY(QFile(filePath).link(symlink)); | 1577 | QVERIFY(QFile(filePath).link(symlink)); | ||
1515 | QVERIFY(QFile::exists(symlink)); | 1578 | QVERIFY(QFile::exists(symlink)); | ||
▲ Show 20 Lines • Show All 741 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.