QFileSystemWatcher: lock autotest code away into a cold section

The code contained a sizeable chunk of string parsing along with
qDebug()s in the normal path of execution. That code, however, was
only used for Qt's own autotests.

The idea of this patch is, then, to not only move the autotest case
into the cold text section (using Q_UNLIKELY), but also to completely
exclude it, when QT_BUILD_INTERNAL is not set.

Unfortunately, the structure of the function did not really lend
itself to #ifdefing that part of the code out (production code was in
the middle of non-production code), so I transformed the engine
selection code into a lambda, replacing assignment with returns, and
swapping the branches of the central if around to yield a single block
of code that can be excluded from compilation with just one #ifdef.

As a consequence, the runtime code is almost unaffected, and the
function is much easier to read now.

Since the test-specific code is only compiled into Qt now in developer
builds, guard the tests that rely on this behavior with the same macro.

Change-Id: I9fd1c57020a13cef4cd1b1674ed2d3ab9424d7cd
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

src/corelib/io/qfilesystemwatcher.cpp

@@ -352,33 +352,34 @@ QStringList QFileSystemWatcher::addPaths(const QStringList &paths)
         return QStringList();
     }
 
-    QFileSystemWatcherEngine *engine = 0;
+    const auto selectEngine = [this, d]() -> QFileSystemWatcherEngine* {
+#ifdef QT_BUILD_INTERNAL
+        const QString on = objectName();
 
-    const QString on = objectName();
-
-    if (!on.startsWith(QLatin1String("_qt_autotest_force_engine_"))) {
+        if (Q_UNLIKELY(on.startsWith(QLatin1String("_qt_autotest_force_engine_")))) {
+            // Autotest override case - use the explicitly selected engine only
+            const QStringRef forceName = on.midRef(26);
+            if (forceName == QLatin1String("poller")) {
+                qDebug("QFileSystemWatcher: skipping native engine, using only polling engine");
+                d_func()->initPollerEngine();
+                return d->poller;
+            } else if (forceName == QLatin1String("native")) {
+                qDebug("QFileSystemWatcher: skipping polling engine, using only native engine");
+                return d->native;
+            }
+            return nullptr;
+        }
+#endif
         // Normal runtime case - search intelligently for best engine
         if(d->native) {
-            engine = d->native;
+            return d->native;
         } else {
             d_func()->initPollerEngine();
-            engine = d->poller;
+            return d->poller;
         }
+    };
 
-    } else {
-        // Autotest override case - use the explicitly selected engine only
-        const QStringRef forceName = on.midRef(26);
-        if(forceName == QLatin1String("poller")) {
-            qDebug("QFileSystemWatcher: skipping native engine, using only polling engine");
-            d_func()->initPollerEngine();
-            engine = d->poller;
-        } else if(forceName == QLatin1String("native")) {
-            qDebug("QFileSystemWatcher: skipping polling engine, using only native engine");
-            engine = d->native;
-        }
-    }
-
-    if(engine)
+    if (auto engine = selectEngine())
         p = engine->addPaths(p, &d->files, &d->directories);
 
     return p;

tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp

@@ -46,11 +46,13 @@ public:
     tst_QFileSystemWatcher();
 
 private slots:
+#ifdef QT_BUILD_INTERNAL
     void basicTest_data();
     void basicTest();
 
     void watchDirectory_data();
     void watchDirectory();
+#endif
 
     void addPath();
     void removePath();
@@ -58,8 +60,10 @@ private slots:
     void removePaths();
     void removePathsFilesInSameDirectory();
 
+#ifdef QT_BUILD_INTERNAL
     void watchFileAndItsDirectory_data() { basicTest_data(); }
     void watchFileAndItsDirectory();
+#endif
 
     void nonExistingFile();
 
@@ -67,8 +71,10 @@ private slots:
 
     void destroyAfterQCoreApplication();
 
+#ifdef QT_BUILD_INTERNAL
     void QTBUG2331();
     void QTBUG2331_data() { basicTest_data(); }
+#endif
 
     void signalsEmittedAfterFileMoved();
 
@@ -90,6 +96,7 @@ tst_QFileSystemWatcher::tst_QFileSystemWatcher()
 #endif
 }
 
+#ifdef QT_BUILD_INTERNAL
 void tst_QFileSystemWatcher::basicTest_data()
 {
     QTest::addColumn<QString>("backend");
@@ -360,6 +367,7 @@ void tst_QFileSystemWatcher::watchDirectory()
     for (const auto &testDirName : testDirs)
         QVERIFY(temporaryDir.rmdir(testDirName));
 }
+#endif // QT_BUILD_INTERNAL
 
 void tst_QFileSystemWatcher::addPath()
 {
@@ -502,6 +510,7 @@ void tst_QFileSystemWatcher::removePathsFilesInSameDirectory()
     QCOMPARE(watcher.files().size(), 0);
 }
 
+#ifdef QT_BUILD_INTERNAL
 static QByteArray msgFileOperationFailed(const char *what, const QFile &f)
 {
     return what + QByteArrayLiteral(" failed on \"")
@@ -601,6 +610,7 @@ void tst_QFileSystemWatcher::watchFileAndItsDirectory()
 
     QVERIFY(temporaryDir.rmdir(testDirName));
 }
+#endif // QT_BUILD_INTERNAL
 
 void tst_QFileSystemWatcher::nonExistingFile()
 {
@@ -673,6 +683,7 @@ void tst_QFileSystemWatcher::destroyAfterQCoreApplication()
     QTest::qWait(30);
 }
 
+#ifdef QT_BUILD_INTERNAL
 // regression test for QTBUG2331.
 // essentially, on windows, directories were not unwatched after being deleted
 // from the disk, causing all sorts of interesting problems.
@@ -696,6 +707,7 @@ void tst_QFileSystemWatcher::QTBUG2331()
     QTRY_COMPARE(changedSpy.count(), 1);
     QCOMPARE(watcher.directories(), QStringList());
 }
+#endif // QT_BUILD_INTERNAL
 
 class SignalReceiver : public QObject
 {