Changeset View
Standalone View
autotests/jobtest.cpp
Show First 20 Lines • Show All 72 Lines • ▼ Show 20 Line(s) | |||||
73 | void JobTest::initTestCase() | 73 | void JobTest::initTestCase() | ||
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 | // to make sure io is not too fast | ||||
82 | qputenv("KIOSLAVE_FILE_ENABLE_TESTMODE", "1"); | ||||
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 | | ||||
81 | s_referenceTimeStamp = QDateTime::currentDateTime().addSecs(-30); // 30 seconds ago | 84 | s_referenceTimeStamp = QDateTime::currentDateTime().addSecs(-30); // 30 seconds ago | ||
82 | 85 | | |||
83 | // Start with a clean base dir | 86 | // Start with a clean base dir | ||
84 | cleanupTestCase(); | 87 | cleanupTestCase(); | ||
85 | homeTmpDir(); // create it | 88 | homeTmpDir(); // create it | ||
86 | if (!QFile::exists(otherTmpDir())) { | 89 | if (!QFile::exists(otherTmpDir())) { | ||
87 | bool ok = QDir().mkdir(otherTmpDir()); | 90 | bool ok = QDir().mkdir(otherTmpDir()); | ||
88 | if (!ok) { | 91 | if (!ok) { | ||
▲ Show 20 Lines • Show All 1989 Lines • ▼ Show 20 Line(s) | |||||
2078 | 2081 | | |||
2079 | void JobTest::cancelCopyAndCleanDest() | 2082 | void JobTest::cancelCopyAndCleanDest() | ||
2080 | { | 2083 | { | ||
2081 | QFETCH(bool, suspend); | 2084 | QFETCH(bool, suspend); | ||
2082 | QFETCH(bool, overwrite); | 2085 | QFETCH(bool, overwrite); | ||
2083 | 2086 | | |||
2084 | const QString baseDir = homeTmpDir(); | 2087 | const QString baseDir = homeTmpDir(); | ||
2085 | const QString srcTemplate = baseDir + QStringLiteral("testfile_XXXXXX"); | 2088 | const QString srcTemplate = baseDir + QStringLiteral("testfile_XXXXXX"); | ||
2086 | const QString destFile = baseDir + QStringLiteral("testfile_copy_") + QString::fromLatin1(QTest::currentDataTag()); | 2089 | const QString destFile = baseDir + QStringLiteral("testfile_copy_slow_") + QString::fromLatin1(QTest::currentDataTag()); | ||
2087 | 2090 | | |||
2088 | QTemporaryFile f(srcTemplate); | 2091 | QTemporaryFile f(srcTemplate); | ||
2089 | if (!f.open()) { | 2092 | if (!f.open()) { | ||
2090 | qFatal("Couldn't open %s", qPrintable(f.fileName())); | 2093 | qFatal("Couldn't open %s", qPrintable(f.fileName())); | ||
2091 | } | 2094 | } | ||
2092 | f.seek(9999999); | 2095 | f.seek(999999); | ||
2093 | f.write("0"); | 2096 | f.write("0"); | ||
2094 | f.close(); | 2097 | f.close(); | ||
2095 | QCOMPARE(f.size(), 10000000); //~10MB | 2098 | QCOMPARE(f.size(), 1000000); //~1MB | ||
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. | |||||
2096 | 2099 | | |||
2097 | if (overwrite) { | 2100 | if (overwrite) { | ||
2098 | createTestFile(destFile); | 2101 | createTestFile(destFile); | ||
2099 | } | 2102 | } | ||
2103 | const QString destToCheck = (overwrite) ? destFile + QStringLiteral(".part") : destFile; | ||||
2100 | 2104 | | |||
2101 | KIO::JobFlag m_overwriteFlag = overwrite ? KIO::Overwrite : KIO::DefaultFlags; | 2105 | KIO::JobFlag m_overwriteFlag = overwrite ? KIO::Overwrite : KIO::DefaultFlags; | ||
2102 | KIO::FileCopyJob *copyJob = KIO::file_copy(QUrl::fromLocalFile(f.fileName()), QUrl::fromLocalFile(destFile), -1, KIO::HideProgressInfo | m_overwriteFlag); | 2106 | KIO::FileCopyJob *copyJob = KIO::file_copy(QUrl::fromLocalFile(f.fileName()), QUrl::fromLocalFile(destFile), -1, KIO::HideProgressInfo | m_overwriteFlag); | ||
2103 | copyJob->setUiDelegate(nullptr); | 2107 | copyJob->setUiDelegate(nullptr); | ||
2104 | QSignalSpy spyProcessedSize(copyJob, &KIO::Job::processedSize); | 2108 | QSignalSpy spyProcessedSize(copyJob, &KIO::Job::processedSize); | ||
2105 | connect(copyJob, &KIO::Job::processedSize, this, [destFile, suspend, overwrite](KJob *job, qulonglong processedSize) { | 2109 | QSignalSpy spyFinished(copyJob, &KIO::Job::finished); | ||
2110 | connect(copyJob, &KIO::Job::processedSize, this, [destFile, suspend, destToCheck](KJob *job, qulonglong processedSize) { | ||||
2106 | if (processedSize > 0) { | 2111 | if (processedSize > 0) { | ||
2107 | const QString destToCheck = (!overwrite) ? destFile : destFile + QStringLiteral(".part"); | | |||
2108 | QVERIFY2(QFile::exists(destToCheck), qPrintable(destToCheck)); | 2112 | QVERIFY2(QFile::exists(destToCheck), qPrintable(destToCheck)); | ||
2109 | if (suspend) { | 2113 | if (suspend) { | ||
2110 | job->suspend(); | 2114 | job->suspend(); | ||
2111 | } | 2115 | } | ||
2112 | job->kill(); | 2116 | QVERIFY(job->kill()); | ||
2113 | QVERIFY(!QFile::exists(destToCheck)); | | |||
2114 | } | 2117 | } | ||
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… | |||||
2115 | }); | 2118 | }); | ||
2119 | | ||||
2116 | QVERIFY(!copyJob->exec()); | 2120 | QVERIFY(!copyJob->exec()); | ||
2117 | QCOMPARE(spyProcessedSize.count(), 1); | 2121 | QCOMPARE(spyProcessedSize.count(), 1); | ||
2118 | // Give time to the kioslave to finish copy() and warn about chown/chmod failing (because FileCopyJob::doKill removed the file) | 2122 | QCOMPARE(spyFinished.count(), 1); | ||
2119 | // Less confusing if the warnings show here. | 2123 | QCOMPARE(copyJob->error(), KIO::ERR_USER_CANCELED); | ||
2120 | QTest::qWait(500); | 2124 | | ||
2121 | QVERIFY(!QFile::exists(destFile)); | 2125 | // the destination file actual deletion happens after finished() is emitted | ||
2126 | // we need to give some time to the ioslave to finish the file cleaning | ||||
2127 | QTRY_VERIFY2(!QFile::exists(destToCheck), qPrintable(destToCheck)); | ||||
2122 | } | 2128 | } | ||
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… | |||||
2123 | 2129 | | |||
meven: This is not right. It might hide a bug. |
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.