Changeset View
Standalone View
autotests/jobtest.cpp
Show First 20 Lines • Show All 73 Lines • ▼ Show 20 Line(s) | |||||
74 | { | 74 | { | ||
75 | QStandardPaths::setTestModeEnabled(true); | 75 | QStandardPaths::setTestModeEnabled(true); | ||
76 | QCoreApplication::instance()->setApplicationName("kio/jobtest"); // testing for #357499 | 76 | QCoreApplication::instance()->setApplicationName("kio/jobtest"); // testing for #357499 | ||
77 | 77 | | |||
78 | // To avoid a runtime dependency on klauncher | 78 | // To avoid a runtime dependency on klauncher | ||
79 | qputenv("KDE_FORK_SLAVES", "yes"); | 79 | qputenv("KDE_FORK_SLAVES", "yes"); | ||
80 | 80 | | |||
81 | s_referenceTimeStamp = QDateTime::currentDateTime().addSecs(-30); // 30 seconds ago | 81 | s_referenceTimeStamp = QDateTime::currentDateTime().addSecs(-30); // 30 seconds ago | ||
82 | 82 | | |||
meven: It'd be nice to have an #idef BUILD_TEST to only include this code path only when tests are… | |||||
But then it could cause difficulties for downstream distros, running tests as part of their packaging process. meven: But then it could cause difficulties for downstream distros, running tests as part of their… | |||||
83 | // Start with a clean base dir | 83 | // Start with a clean base dir | ||
84 | cleanupTestCase(); | 84 | cleanupTestCase(); | ||
85 | homeTmpDir(); // create it | 85 | homeTmpDir(); // create it | ||
86 | if (!QFile::exists(otherTmpDir())) { | 86 | if (!QFile::exists(otherTmpDir())) { | ||
87 | bool ok = QDir().mkdir(otherTmpDir()); | 87 | bool ok = QDir().mkdir(otherTmpDir()); | ||
88 | if (!ok) { | 88 | if (!ok) { | ||
89 | qFatal("couldn't create %s", qPrintable(otherTmpDir())); | 89 | qFatal("couldn't create %s", qPrintable(otherTmpDir())); | ||
90 | } | 90 | } | ||
▲ Show 20 Lines • Show All 1946 Lines • ▼ Show 20 Line(s) | 2030 | { | |||
2037 | 2037 | | |||
2038 | QTemporaryFile f(srcTemplate); | 2038 | QTemporaryFile f(srcTemplate); | ||
2039 | if (!f.open()) { | 2039 | if (!f.open()) { | ||
2040 | qFatal("Couldn't open %s", qPrintable(f.fileName())); | 2040 | qFatal("Couldn't open %s", qPrintable(f.fileName())); | ||
2041 | } | 2041 | } | ||
2042 | f.seek(9999999); | 2042 | f.seek(9999999); | ||
2043 | f.write("0"); | 2043 | f.write("0"); | ||
2044 | f.close(); | 2044 | f.close(); | ||
2045 | QCOMPARE(f.size(), 10000000); //~10MB | 2045 | QCOMPARE(f.size(), 10000000); //~10MB to make sure copy is not too fast, or the tests won't pass | ||
meven: This may-be excessive, but I needed that since I have a fast nvme drive. | |||||
The alternative is to export an env var that makes the ioslave call some msleep()... dfaure: The alternative is to export an env var that makes the ioslave call some msleep()... | |||||
Making an ioslave reading an env var for testing purposes is not a great alternative. meven: Making an ioslave reading an env var for testing purposes is not a great alternative. | |||||
Does this mean the file could be made smaller again? (to reduce disk-space requirements) dfaure: Does this mean the file could be made smaller again?
(to reduce disk-space requirements) | |||||
I have reduced the size space needed to a smaller value than before this patch. meven: I have reduced the size space needed to a smaller value than before this patch. | |||||
2046 | 2046 | | |||
2047 | if (overwrite) { | 2047 | if (overwrite) { | ||
2048 | createTestFile(destFile); | 2048 | createTestFile(destFile); | ||
2049 | } | 2049 | } | ||
2050 | const QString destToCheck = (!overwrite) ? destFile : destFile + QStringLiteral(".part"); | ||||
2050 | 2051 | | |||
2051 | KIO::JobFlag m_overwriteFlag = overwrite ? KIO::Overwrite : KIO::DefaultFlags; | 2052 | KIO::JobFlag m_overwriteFlag = overwrite ? KIO::Overwrite : KIO::DefaultFlags; | ||
2052 | KIO::FileCopyJob *copyJob = KIO::file_copy(QUrl::fromLocalFile(f.fileName()), QUrl::fromLocalFile(destFile), -1, KIO::HideProgressInfo | m_overwriteFlag); | 2053 | KIO::FileCopyJob *copyJob = KIO::file_copy(QUrl::fromLocalFile(f.fileName()), QUrl::fromLocalFile(destFile), -1, KIO::HideProgressInfo | m_overwriteFlag); | ||
2053 | copyJob->setUiDelegate(nullptr); | 2054 | copyJob->setUiDelegate(nullptr); | ||
2054 | QSignalSpy spyProcessedSize(copyJob, &KIO::Job::processedSize); | 2055 | QSignalSpy spyProcessedSize(copyJob, &KIO::Job::processedSize); | ||
2055 | connect(copyJob, &KIO::Job::processedSize, this, [destFile, suspend, overwrite](KJob *job, qulonglong processedSize) { | 2056 | QSignalSpy spyFinished(copyJob, &KIO::Job::finished); | ||
2057 | connect(copyJob, &KIO::Job::processedSize, this, [destFile, suspend, destToCheck](KJob *job, qulonglong processedSize) { | ||||
2056 | if (processedSize > 0) { | 2058 | if (processedSize > 0) { | ||
2057 | const QString destToCheck = (!overwrite) ? destFile : destFile + QStringLiteral(".part"); | | |||
2058 | QVERIFY2(QFile::exists(destToCheck), qPrintable(destToCheck)); | 2059 | QVERIFY2(QFile::exists(destToCheck), qPrintable(destToCheck)); | ||
2059 | if (suspend) { | 2060 | if (suspend) { | ||
2060 | job->suspend(); | 2061 | job->suspend(); | ||
2061 | } | 2062 | } | ||
2062 | job->kill(); | 2063 | QVERIFY(job->kill()); | ||
2063 | QVERIFY(!QFile::exists(destToCheck)); | | |||
2064 | } | 2064 | } | ||
Always use QVERIFY() around spy.wait(). dfaure: Always use QVERIFY() around spy.wait().
Well, I'm assuming we actually expect the signal to be… | |||||
dfaure: Why the short 500ms timeout? CI can be slow sometimes. | |||||
It was the previous value used line 2176 for the same use, except the file cleaning will now be more immediate. meven: It was the previous value used line 2176 for the same use, except the file cleaning will now be… | |||||
2065 | }); | 2065 | }); | ||
2066 | | ||||
2066 | QVERIFY(!copyJob->exec()); | 2067 | QVERIFY(!copyJob->exec()); | ||
2067 | QCOMPARE(spyProcessedSize.count(), 1); | 2068 | QCOMPARE(spyProcessedSize.count(), 1); | ||
2068 | // Give time to the kioslave to finish copy() and warn about chown/chmod failing (because FileCopyJob::doKill removed the file) | 2069 | QCOMPARE(spyFinished.count(), 1); | ||
2069 | // Less confusing if the warnings show here. | 2070 | QCOMPARE(copyJob->error(), KIO::ERR_USER_CANCELED); | ||
2070 | QTest::qWait(500); | 2071 | | ||
2071 | QVERIFY(!QFile::exists(destFile)); | 2072 | if (overwrite) { | ||
2073 | // the destination file actual deletion happens after finished() is emitted | ||||
2074 | // we need to give some time to the ioslave to finish the file cleaning | ||||
2075 | QTRY_VERIFY(!QFile::exists(destToCheck)); | ||||
How about QTRY_VERIFY instead of a hardcoded 500ms sleep? This way, if the file gets deleted after 100ms, we can move on. dfaure: How about QTRY_VERIFY instead of a hardcoded 500ms sleep?
This way, if the file gets deleted… | |||||
2076 | } else { | ||||
2077 | QVERIFY(QFile::exists(destToCheck)); | ||||
meven: This is not right. It might hide a bug. | |||||
2078 | } | ||||
2072 | } | 2079 | } | ||
2073 | 2080 | |
It'd be nice to have an #idef BUILD_TEST to only include this code path only when tests are compiled.
I didn't know how to achieve this.