From d2f117439f93b9c2045c1475a1298f87ece65994 Mon Sep 17 00:00:00 2001
From: Samuel Jackson <samueljackson@outlook.com>
Date: Tue, 17 Oct 2017 16:11:02 +0100
Subject: [PATCH] Refs #20855 Begin to get thread manager under test
---
qt/widgets/common/CMakeLists.txt | 7 ++-
.../Common/FindFilesThreadPoolManager.h | 1 +
.../FindFilesThreadPoolManagerMockObjects.h | 45 ++++++++++++++++++
.../common/src/FindFilesThreadPoolManager.cpp | 18 +++++--
qt/widgets/common/src/MWRunFiles.cpp | 2 +-
.../test/FindFilesThreadPoolManagerTest.h | 47 +++++++++++++++++++
6 files changed, 112 insertions(+), 8 deletions(-)
create mode 100644 qt/widgets/common/inc/MantidQtWidgets/Common/FindFilesThreadPoolManagerMockObjects.h
create mode 100644 qt/widgets/common/test/FindFilesThreadPoolManagerTest.h
diff --git a/qt/widgets/common/CMakeLists.txt b/qt/widgets/common/CMakeLists.txt
index 4cfe6557a00..d040c9eb429 100644
--- a/qt/widgets/common/CMakeLists.txt
+++ b/qt/widgets/common/CMakeLists.txt
@@ -203,7 +203,6 @@ set ( MOC_FILES
inc/MantidQtWidgets/Common/DoubleSpinBox.h
inc/MantidQtWidgets/Common/FindReplaceDialog.h
inc/MantidQtWidgets/Common/FindDialog.h
- inc/MantidQtWidgets/Common/FindFilesThreadPoolManager.h
inc/MantidQtWidgets/Common/FitPropertyBrowser.h
inc/MantidQtWidgets/Common/FitOptionsBrowser.h
inc/MantidQtWidgets/Common/FunctionBrowser.h
@@ -255,6 +254,8 @@ set ( MOC_FILES
inc/MantidQtWidgets/Common/QtPropertyBrowser/StringDialogEditor.h
inc/MantidQtWidgets/Common/QtPropertyBrowser/StringEditorFactory.h
inc/MantidQtWidgets/Common/QtPropertyBrowser/WorkspaceEditorFactory.h
+ inc/MantidQtWidgets/Common/FindFilesThreadPoolManager.h
+ inc/MantidQtWidgets/Common/FindFilesThreadPoolManagerMockObjects.h
)
# Include files aren't required, but this makes them appear in Visual Studio
@@ -393,7 +394,8 @@ set ( INC_FILES
inc/MantidQtWidgets/Common/QtPropertyBrowser/qtpropertymanager.h
inc/MantidQtWidgets/Common/QtPropertyBrowser/qttreepropertybrowser.h
inc/MantidQtWidgets/Common/QtPropertyBrowser/qtvariantproperty.h
-
+ inc/MantidQtWidgets/Common/FindFilesThreadPoolManager.h
+ inc/MantidQtWidgets/Common/FindFilesThreadPoolManagerMockObjects.h
)
set ( UI_FILES
@@ -433,6 +435,7 @@ set ( TEST_PY_FILES
set( TEST_FILES
BatchAlgorithmRunnerTest.h
FileDialogHandlerTest.h
+ FindFilesThreadPoolManagerTest.h
InterfaceManagerTest.h
MantidColorMapTest.h
NonOrthogonalTest.h
diff --git a/qt/widgets/common/inc/MantidQtWidgets/Common/FindFilesThreadPoolManager.h b/qt/widgets/common/inc/MantidQtWidgets/Common/FindFilesThreadPoolManager.h
index a36ee8268ca..3965d6371e3 100644
--- a/qt/widgets/common/inc/MantidQtWidgets/Common/FindFilesThreadPoolManager.h
+++ b/qt/widgets/common/inc/MantidQtWidgets/Common/FindFilesThreadPoolManager.h
@@ -96,6 +96,7 @@ public:
const FindFilesSearchParameters& parameters);
void cancelWorker(const QObject *parent);
bool isSearchRunning() const;
+ void waitForDone() const;
private:
// make a local thread pool
diff --git a/qt/widgets/common/inc/MantidQtWidgets/Common/FindFilesThreadPoolManagerMockObjects.h b/qt/widgets/common/inc/MantidQtWidgets/Common/FindFilesThreadPoolManagerMockObjects.h
new file mode 100644
index 00000000000..2f4fbc31a09
--- /dev/null
+++ b/qt/widgets/common/inc/MantidQtWidgets/Common/FindFilesThreadPoolManagerMockObjects.h
@@ -0,0 +1,45 @@
+#ifndef MANTIDQT_API_FINDFILESTHREADPOOLMANAGERTESTMOCKOBJECTS_H_
+#define MANTIDQT_API_FINDFILESTHREADPOOLMANAGERTESTMOCKOBJECTS_H_
+
+#include "FindFilesThreadPoolManager.h"
+
+#include <QObject>
+
+namespace MantidQt {
+namespace API {
+
+class FakeMWRunFiles : public QObject {
+Q_OBJECT
+public slots:
+ /// Slot called when file finding thread has finished.
+ void inspectThreadResult(const FindFilesSearchResults& result) {
+ m_results = result;
+ }
+
+ /// Slot called when the file finding thread has finished.
+ void setSignalRecieved() {
+ m_finishedSignalRecieved = true;
+ }
+
+signals:
+ void fileFindingFinished();
+
+public:
+ FakeMWRunFiles() : m_results(), m_finishedSignalRecieved(false) {
+ connect(this, SIGNAL(fileFindingFinished()),
+ this, SLOT(setSignalRecieved()),
+ Qt::DirectConnection);
+ }
+
+ FindFilesSearchResults getResults() { return m_results; };
+ bool isFinishedSignalRecieved() { return m_finishedSignalRecieved; };
+
+private:
+ FindFilesSearchResults m_results;
+ bool m_finishedSignalRecieved;
+};
+
+}
+}
+
+#endif /* MANTIDQT_API_FINDFILESTHREADPOOLMANAGERTESTMOCKOBJECTS_H_ */
diff --git a/qt/widgets/common/src/FindFilesThreadPoolManager.cpp b/qt/widgets/common/src/FindFilesThreadPoolManager.cpp
index 6f85f6ac525..5cd5b2971f7 100644
--- a/qt/widgets/common/src/FindFilesThreadPoolManager.cpp
+++ b/qt/widgets/common/src/FindFilesThreadPoolManager.cpp
@@ -125,7 +125,7 @@ void FindFilesThread::run() {
m_filenames.clear();
}
- const auto result = createFindFilesSearchResult();
+ auto result = createFindFilesSearchResult();
emit finished(result);
}
@@ -196,13 +196,18 @@ void FindFilesThreadPoolManager::createWorker(
m_currentWorker = new FindFilesThread();
- // Hook up slots for when the thread finishes
+ // Hook up slots for when the thread finishes. By default Qt uses queued
+ // connections when connecting signals/slots between threads. Instead here
+ // we explicitly choose to use a direct connection so the found result is
+ // immediately returned to the GUI thread.
parent->connect(m_currentWorker,
SIGNAL(finished(const FindFilesSearchResults &)), parent,
- SLOT(inspectThreadResult(const FindFilesSearchResults &)));
+ SLOT(inspectThreadResult(const FindFilesSearchResults &)),
+ Qt::DirectConnection);
parent->connect(m_currentWorker,
SIGNAL(finished(const FindFilesSearchResults &)), parent,
- SIGNAL(fileFindingFinished()));
+ SIGNAL(fileFindingFinished()),
+ Qt::DirectConnection);
// Set the search parameters
m_currentWorker->set(parameters);
@@ -219,10 +224,13 @@ void FindFilesThreadPoolManager::cancelWorker(const QObject *parent) {
// Just disconnect any signals from the worker. We leave the worker to
// continue running in the background because 1) terminating it directly
// is dangerous (we have no idea what it's currently doing from here) and 2)
- // waiting for it to finish locks up the GUI event loop.
+ // waiting for it to finish before starting a new thread locks up the GUI
+ // event loop.
m_currentWorker->disconnect(parent);
}
bool FindFilesThreadPoolManager::isSearchRunning() const {
return m_currentWorker != nullptr;
}
+
+void FindFilesThreadPoolManager::waitForDone() const { m_pool.waitForDone(); }
diff --git a/qt/widgets/common/src/MWRunFiles.cpp b/qt/widgets/common/src/MWRunFiles.cpp
index f3bede0f402..d927933f3a4 100644
--- a/qt/widgets/common/src/MWRunFiles.cpp
+++ b/qt/widgets/common/src/MWRunFiles.cpp
@@ -572,7 +572,7 @@ void MWRunFiles::findFiles() {
if (!searchText.isEmpty()) {
const auto parameters = createFindFilesSearchParameters(searchText);
- m_pool.createWorker(this, parameters);
+ // m_pool.createWorker(this, parameters);
}
} else {
diff --git a/qt/widgets/common/test/FindFilesThreadPoolManagerTest.h b/qt/widgets/common/test/FindFilesThreadPoolManagerTest.h
new file mode 100644
index 00000000000..b23a1b1c758
--- /dev/null
+++ b/qt/widgets/common/test/FindFilesThreadPoolManagerTest.h
@@ -0,0 +1,47 @@
+#ifndef MANTIDQT_API_FINDFILESTHREADPOOLMANAGERTEST_H_
+#define MANTIDQT_API_FINDFILESTHREADPOOLMANAGERTEST_H_
+
+#include "MantidQtWidgets/Common/FindFilesThreadPoolManager.h"
+#include "MantidQtWidgets/Common/FindFilesThreadPoolManagerMockObjects.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <cxxtest/TestSuite.h>
+
+using MantidQt::API::FindFilesThreadPoolManager;
+using MantidQt::API::FindFilesSearchParameters;
+using MantidQt::API::FindFilesSearchResults;
+using MantidQt::API::FakeMWRunFiles;
+
+class FindFilesThreadPoolManagerTest : public CxxTest::TestSuite {
+public:
+ // This pair of boilerplate methods prevent the suite being created statically
+ // This means the constructor isn't called when running other tests
+ static FindFilesThreadPoolManagerTest *createSuite() { return new FindFilesThreadPoolManagerTest(); }
+ static void destroySuite(FindFilesThreadPoolManagerTest *suite) { delete suite; }
+
+ void test_find_single_file() {
+ // Arrange
+ FakeMWRunFiles widget;
+ FindFilesSearchParameters parameters;
+ parameters.searchText = "SomeFileThatDontExist";
+ parameters.isOptional = false;
+ parameters.isForRunFiles = false;
+ parameters.algorithmProperty = "Filename";
+ parameters.algorithmName = "Load";
+
+ // Act
+ FindFilesThreadPoolManager poolManager;
+ poolManager.createWorker(&widget, parameters);
+ // Block and wait for all the threads to process
+ poolManager.waitForDone();
+
+ // Assert
+ const auto results = widget.getResults();
+ TS_ASSERT(widget.isFinishedSignalRecieved())
+ TS_ASSERT_EQUALS(results.error, "")
+ }
+
+};
+
+#endif /* MANTIDQT_API_FINDFILESTHREADPOOLMANAGERTEST */
--
GitLab