diff --git a/qt/widgets/common/CMakeLists.txt b/qt/widgets/common/CMakeLists.txt index 4cfe6557a00c9ae2b8d843ba08a20c5520a175c0..d040c9eb42921ac5b1e30f1a07546b594fe837bb 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 a36ee8268caccff5bcc3d9c2d8221279b8ea9c0f..3965d6371e35dcd0453f6d3e36c3c3860ae243dc 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 0000000000000000000000000000000000000000..2f4fbc31a09e1a339a7a7224a4449a5fc05e9281 --- /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 6f85f6ac52552bd78893b22b18021ea7e2a3193c..5cd5b2971f7c5aac816ab07e13967a4eaf469f17 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 f3bede0f40251ca82ace31bd8574d7d17de0b80f..d927933f3a4abe10134558e5ecf20f34921182e2 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 0000000000000000000000000000000000000000..b23a1b1c75845ccc4fafe0aa25858b0db837a550 --- /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 */