From 315b359bc199fcc9492fa33f573d6c43d22b3b6a Mon Sep 17 00:00:00 2001
From: Friedemann Kleint <Friedemann.Kleint@theqtcompany.com>
Date: Tue, 28 Apr 2015 08:44:53 +0200
Subject: [PATCH] QFileDialog: Stabilize tests.

The init()/cleanup() code in tst_qfiledialog and tst_qfiledialog2
currently differs and fails to clean up the settings file
since it only removes the legacy settings under the Qt group
and instantiates a new QFileDialog while the QSettings class
is still in scope. Also, it has no means of clearing the
setLastVisitedDirectory(), which causes the
tst_QFiledialog::completer() and tst_QFiledialog::history() tests
to interfere, leaving the settings in an invalid state.
tst_qfiledialog2 does not use QStandardPaths::setTestModeEnabled(().

- Ensure the last visited URL is always clean by
  making QFileDialogPrivate::setLastVisitedDirectory()
  static and calling it from init().
- Introduce a cleanupSettingsFile() function to the tests that
  cleans both groups and call it from initTestCase() and cleanup()
  to ensure a clean state.
- Add QStandardPaths::setTestModeEnabled() to tst_qfiledialog2.

Fixes sporadic test fails when executing
tst_QFiledialog::completer() and tst_QFiledialog::history()
in a sequence.

Task-number: QTBUG-45764
Change-Id: I24de3caabf77be067b385d64ff11b7a07fe12b72
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>
---
 src/widgets/dialogs/qfiledialog.cpp           |  4 +-
 src/widgets/dialogs/qfiledialog_p.h           |  2 +-
 .../dialogs/qfiledialog/tst_qfiledialog.cpp   | 29 +++++++++++----
 .../dialogs/qfiledialog2/tst_qfiledialog2.cpp | 37 ++++++++++++-------
 4 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/src/widgets/dialogs/qfiledialog.cpp b/src/widgets/dialogs/qfiledialog.cpp
index 9e5548ab4cc..8590467abcb 100644
--- a/src/widgets/dialogs/qfiledialog.cpp
+++ b/src/widgets/dialogs/qfiledialog.cpp
@@ -906,7 +906,7 @@ void QFileDialog::setDirectory(const QString &directory)
         return;
 
     QUrl newDirUrl = QUrl::fromLocalFile(newDirectory);
-    d->setLastVisitedDirectory(newDirUrl);
+    QFileDialogPrivate::setLastVisitedDirectory(newDirUrl);
 
     d->options->setInitialDirectory(QUrl::fromLocalFile(directory));
     if (!d->usingWidgets()) {
@@ -967,7 +967,7 @@ void QFileDialog::setDirectoryUrl(const QUrl &directory)
     if (!directory.isValid())
         return;
 
-    d->setLastVisitedDirectory(directory);
+    QFileDialogPrivate::setLastVisitedDirectory(directory);
     d->options->setInitialDirectory(directory);
 
     if (d->nativeDialogInUse)
diff --git a/src/widgets/dialogs/qfiledialog_p.h b/src/widgets/dialogs/qfiledialog_p.h
index 8a108969665..f610e46f83f 100644
--- a/src/widgets/dialogs/qfiledialog_p.h
+++ b/src/widgets/dialogs/qfiledialog_p.h
@@ -187,7 +187,7 @@ public:
 #endif
 
     bool restoreWidgetState(QStringList &history, int splitterPosition);
-    void setLastVisitedDirectory(const QUrl &dir);
+    static void setLastVisitedDirectory(const QUrl &dir);
     void retranslateWindowTitle();
     void retranslateStrings();
     void emitFilesSelected(const QStringList &files);
diff --git a/tests/auto/widgets/dialogs/qfiledialog/tst_qfiledialog.cpp b/tests/auto/widgets/dialogs/qfiledialog/tst_qfiledialog.cpp
index e677891ce5e..ffc000a4182 100644
--- a/tests/auto/widgets/dialogs/qfiledialog/tst_qfiledialog.cpp
+++ b/tests/auto/widgets/dialogs/qfiledialog/tst_qfiledialog.cpp
@@ -54,10 +54,10 @@
 #include <qsortfilterproxymodel.h>
 #include <qlineedit.h>
 #include <qlayout.h>
+#include <private/qfiledialog_p.h>
 #if defined QT_BUILD_INTERNAL
 #include <private/qsidebar_p.h>
 #include <private/qfilesystemmodel_p.h>
-#include <private/qfiledialog_p.h>
 #endif
 #include <private/qguiapplication_p.h>
 #include <qpa/qplatformtheme.h>
@@ -108,6 +108,7 @@ public:
 public slots:
     void initTestCase();
     void init();
+    void cleanup();
 
 private slots:
     void currentChangedSignal();
@@ -166,7 +167,7 @@ private slots:
     void rejectModalDialogs();
 
 private:
-    QByteArray userSettings;
+    void cleanupSettingsFile();
 };
 
 tst_QFiledialog::tst_QFiledialog()
@@ -177,18 +178,27 @@ tst_QFiledialog::~tst_QFiledialog()
 {
 }
 
+void tst_QFiledialog::cleanupSettingsFile()
+{
+    // clean up the sidebar between each test
+    QSettings settings(QSettings::UserScope, QLatin1String("QtProject"));
+    settings.beginGroup(QLatin1String("FileDialog"));
+    settings.remove(QString());
+    settings.endGroup();
+    settings.beginGroup(QLatin1String("Qt")); // Compatibility settings
+    settings.remove(QLatin1String("filedialog"));
+    settings.endGroup();
+}
+
 void tst_QFiledialog::initTestCase()
 {
     QStandardPaths::setTestModeEnabled(true);
+    cleanupSettingsFile();
 }
 
 void tst_QFiledialog::init()
 {
-    // clean up the sidebar between each test
-    QSettings settings(QSettings::UserScope, QLatin1String("QtProject"));
-    settings.beginGroup(QLatin1String("Qt"));
-    settings.remove(QLatin1String("filedialog"));
-
+    QFileDialogPrivate::setLastVisitedDirectory(QUrl());
     // populate the sidebar with some default settings
     QNonNativeFileDialog fd;
 #if defined(Q_OS_WINCE)
@@ -196,6 +206,11 @@ void tst_QFiledialog::init()
 #endif
 }
 
+void tst_QFiledialog::cleanup()
+{
+    cleanupSettingsFile();
+}
+
 class MyAbstractItemDelegate : public QAbstractItemDelegate
 {
 public:
diff --git a/tests/auto/widgets/dialogs/qfiledialog2/tst_qfiledialog2.cpp b/tests/auto/widgets/dialogs/qfiledialog2/tst_qfiledialog2.cpp
index 7af62cb2a19..ed34c67aad3 100644
--- a/tests/auto/widgets/dialogs/qfiledialog2/tst_qfiledialog2.cpp
+++ b/tests/auto/widgets/dialogs/qfiledialog2/tst_qfiledialog2.cpp
@@ -88,6 +88,7 @@ public:
     virtual ~tst_QFileDialog2();
 
 public slots:
+    void initTestCase();
     void init();
     void cleanup();
 
@@ -135,13 +136,13 @@ private slots:
     void dontShowCompleterOnRoot();
 
 private:
-    QByteArray userSettings;
+    void cleanupSettingsFile();
+
     QTemporaryDir tempDir;
 };
 
 tst_QFileDialog2::tst_QFileDialog2()
-    : userSettings()
-    , tempDir(QDir::tempPath() + "/tst_qfiledialog2.XXXXXX")
+    : tempDir(QDir::tempPath() + "/tst_qfiledialog2.XXXXXX")
 {
 #if defined(Q_OS_WINCE)
     qApp->setAutoMaximizeThreshold(-1);
@@ -152,17 +153,29 @@ tst_QFileDialog2::~tst_QFileDialog2()
 {
 }
 
-void tst_QFileDialog2::init()
+void tst_QFileDialog2::cleanupSettingsFile()
 {
-    QVERIFY(tempDir.isValid());
-
-    // Save the developers settings so they don't get mad when their sidebar folders are gone.
+    // clean up the sidebar between each test
     QSettings settings(QSettings::UserScope, QLatin1String("QtProject"));
-    settings.beginGroup(QLatin1String("Qt"));
-    userSettings = settings.value(QLatin1String("filedialog")).toByteArray();
+    settings.beginGroup(QLatin1String("FileDialog"));
+    settings.remove(QString());
+    settings.endGroup();
+    settings.beginGroup(QLatin1String("Qt")); // Compatibility settings
     settings.remove(QLatin1String("filedialog"));
+    settings.endGroup();
+}
+
+void tst_QFileDialog2::initTestCase()
+{
+    QVERIFY(tempDir.isValid());
+    QStandardPaths::setTestModeEnabled(true);
+    cleanupSettingsFile();
+}
 
-    // populate it with some default settings
+void tst_QFileDialog2::init()
+{
+    QFileDialogPrivate::setLastVisitedDirectory(QUrl());
+    // populate the sidebar with some default settings
     QNonNativeFileDialog fd;
 #if defined(Q_OS_WINCE)
     QTest::qWait(1000);
@@ -171,9 +184,7 @@ void tst_QFileDialog2::init()
 
 void tst_QFileDialog2::cleanup()
 {
-    QSettings settings(QSettings::UserScope, QLatin1String("QtProject"));
-    settings.beginGroup(QLatin1String("Qt"));
-    settings.setValue(QLatin1String("filedialog"), userSettings);
+    cleanupSettingsFile();
 }
 
 #ifdef QT_BUILD_INTERNAL
-- 
GitLab