From 9f0c96cf9ae44b1960f5206eb17fad28d2a34d0b Mon Sep 17 00:00:00 2001
From: Gemma Guest <gemma.guest@stfc.ac.uk>
Date: Tue, 22 May 2018 16:04:54 +0100
Subject: [PATCH] Tidy up ProgressableView and add tests
Improve function names, move definitions to a cpp file and add unit tests
Re #17029
---
.../ReflRunsTabPresenter.cpp | 6 +-
.../test/ReflRunsTabPresenterTest.h | 2 +
qt/widgets/common/CMakeLists.txt | 3 +-
.../ProgressableViewMockObject.h | 2 +
.../Common/ProgressPresenter.h | 5 +-
.../MantidQtWidgets/Common/ProgressableView.h | 27 ++----
qt/widgets/common/src/ProgressableView.cpp | 32 +++++++
qt/widgets/common/test/ProgressableViewTest.h | 84 +++++++++++++++++++
8 files changed, 135 insertions(+), 26 deletions(-)
create mode 100644 qt/widgets/common/src/ProgressableView.cpp
create mode 100644 qt/widgets/common/test/ProgressableViewTest.h
diff --git a/qt/scientific_interfaces/ISISReflectometry/ReflRunsTabPresenter.cpp b/qt/scientific_interfaces/ISISReflectometry/ReflRunsTabPresenter.cpp
index e3b871b1524..cf84a378a38 100644
--- a/qt/scientific_interfaces/ISISReflectometry/ReflRunsTabPresenter.cpp
+++ b/qt/scientific_interfaces/ISISReflectometry/ReflRunsTabPresenter.cpp
@@ -502,9 +502,9 @@ ReflRunsTabPresenter::setupProgressBar(const std::set<int> &rowsToTransfer) {
auto progress = ProgressPresenter(start, end, nsteps, this->m_progressView);
if (autoreductionRunning())
- progress.setStyle(ProgressableView::Style::ENDLESS);
+ progress.setAsEndlessIndicator();
else
- progress.setStyle(ProgressableView::Style::PERCENTAGE);
+ progress.setAsPercentageIndicator();
return progress;
}
@@ -696,7 +696,7 @@ void ReflRunsTabPresenter::pause(int group) {
m_view->stopTimer();
updateWidgetEnabledState(false);
- m_progressView->setStyle(ProgressableView::Style::PERCENTAGE);
+ m_progressView->setAsPercentageIndicator();
// We get here in two scenarios: processing is still running, in which case
// do not confirm reduction has paused yet (confirmReductionPaused will be
diff --git a/qt/scientific_interfaces/test/ReflRunsTabPresenterTest.h b/qt/scientific_interfaces/test/ReflRunsTabPresenterTest.h
index 74fee7cff1f..8d8baba4124 100644
--- a/qt/scientific_interfaces/test/ReflRunsTabPresenterTest.h
+++ b/qt/scientific_interfaces/test/ReflRunsTabPresenterTest.h
@@ -293,6 +293,8 @@ public:
.Times(Exactly(1));
EXPECT_CALL(mockRunsTabView, setSearchButtonEnabled(true))
.Times(Exactly(1));
+ EXPECT_CALL(mockProgress, setAsPercentageIndicator())
+ .Times(Exactly(1));
// Pause presenter
presenter.pause(GROUP_NUMBER);
diff --git a/qt/widgets/common/CMakeLists.txt b/qt/widgets/common/CMakeLists.txt
index 806d17317c0..60a626ec6eb 100644
--- a/qt/widgets/common/CMakeLists.txt
+++ b/qt/widgets/common/CMakeLists.txt
@@ -124,6 +124,7 @@ set ( SRC_FILES
src/MuonFunctionBrowser.cpp
src/PeriodicTableWidget.cpp
src/ProcessingAlgoWidget.cpp
+ src/ProgressableView.cpp
src/ProjectSavePresenter.cpp
src/ProjectSaveModel.cpp
src/PropertyHandler.cpp
@@ -376,7 +377,6 @@ set ( INC_FILES
inc/MantidQtWidgets/Common/ProgressPresenter.h
inc/MantidQtWidgets/Common/ProjectSavePresenter.h
inc/MantidQtWidgets/Common/ProjectSaveModel.h
- inc/MantidQtWidgets/Common/ProgressableView.h
inc/MantidQtWidgets/Common/WorkspacePresenter/ADSAdapter.h
inc/MantidQtWidgets/Common/WorkspacePresenter/IWorkspaceDockView.h
inc/MantidQtWidgets/Common/WorkspacePresenter/ViewNotifiable.h
@@ -574,6 +574,7 @@ set( TEST_FILES
test/ParseKeyValueStringTest.h
test/DataProcessorUI/QOneLevelTreeModelTest.h
test/DataProcessorUI/QTwoLevelTreeModelTest.h
+ test/ProgressableViewTest.h
test/ProjectSaveModelTest.h
test/ProjectSavePresenterTest.h
test/WorkspacePresenter/ADSAdapterTest.h
diff --git a/qt/widgets/common/inc/MantidQtWidgets/Common/DataProcessorUI/ProgressableViewMockObject.h b/qt/widgets/common/inc/MantidQtWidgets/Common/DataProcessorUI/ProgressableViewMockObject.h
index 3c151f809af..69b768874d8 100644
--- a/qt/widgets/common/inc/MantidQtWidgets/Common/DataProcessorUI/ProgressableViewMockObject.h
+++ b/qt/widgets/common/inc/MantidQtWidgets/Common/DataProcessorUI/ProgressableViewMockObject.h
@@ -14,6 +14,8 @@ public:
MOCK_METHOD1(setProgress, void(int));
MOCK_METHOD2(setProgressRange, void(int, int));
MOCK_METHOD0(clearProgress, void());
+ MOCK_METHOD0(setAsPercentageIndicator, void());
+ MOCK_METHOD0(setAsEndlessIndicator, void());
GCC_DIAG_ON_SUGGEST_OVERRIDE
};
diff --git a/qt/widgets/common/inc/MantidQtWidgets/Common/ProgressPresenter.h b/qt/widgets/common/inc/MantidQtWidgets/Common/ProgressPresenter.h
index ffeb1005878..26302d3acb3 100644
--- a/qt/widgets/common/inc/MantidQtWidgets/Common/ProgressPresenter.h
+++ b/qt/widgets/common/inc/MantidQtWidgets/Common/ProgressPresenter.h
@@ -28,9 +28,10 @@ public:
m_progressableView->setProgress(static_cast<int>(m_i));
}
void clear() { m_progressableView->clearProgress(); }
- void setStyle(MantidQt::MantidWidgets::ProgressableView::Style style) {
- m_progressableView->setStyle(style);
+ void setAsPercentageIndicator() {
+ m_progressableView->setAsPercentageIndicator();
}
+ void setAsEndlessIndicator() { m_progressableView->setAsEndlessIndicator(); }
~ProgressPresenter() {}
};
#endif /* MANTIDQTMANTIDWIDGETS_PROGRESSPRESENTER_H */
diff --git a/qt/widgets/common/inc/MantidQtWidgets/Common/ProgressableView.h b/qt/widgets/common/inc/MantidQtWidgets/Common/ProgressableView.h
index 35421b04197..73f269a9175 100644
--- a/qt/widgets/common/inc/MantidQtWidgets/Common/ProgressableView.h
+++ b/qt/widgets/common/inc/MantidQtWidgets/Common/ProgressableView.h
@@ -31,6 +31,8 @@ namespace MantidWidgets {
*/
class EXPORT_OPT_MANTIDQT_COMMON ProgressableView {
public:
+ /// The style of the progress bar: either a standard percentage progress bar
+ /// or an endless busy indicator
enum class Style { PERCENTAGE, ENDLESS };
ProgressableView() : m_style{Style::PERCENTAGE}, m_min(0), m_max(100) {}
@@ -38,28 +40,13 @@ public:
virtual void setProgress(int progress) = 0;
virtual void clearProgress() = 0;
+ virtual void setProgressRange(int min, int max);
- virtual void setProgressRange(int min, int max) {
- // Cache values for a percentage-style progress bar i.e. where both are not
- // zero
- if (min != 0 || max != 0) {
- m_min = min;
- m_max = max;
- }
- }
+ bool isPercentageIndicator() const;
+ void setAsPercentageIndicator();
+ void setAsEndlessIndicator();
- bool isPercentageIndicator() { return m_style == Style::PERCENTAGE; }
- void setStyle(Style style) {
- m_style = style;
- // To get QProgressBar to display as an endless progress indicator, we need
- // to set start=end=0
- if (m_style == Style::ENDLESS)
- setProgressRange(0, 0);
- else
- setProgressRange(m_min, m_max);
- };
-
-private:
+protected:
Style m_style;
int m_min;
int m_max;
diff --git a/qt/widgets/common/src/ProgressableView.cpp b/qt/widgets/common/src/ProgressableView.cpp
new file mode 100644
index 00000000000..7b7c9fc3b78
--- /dev/null
+++ b/qt/widgets/common/src/ProgressableView.cpp
@@ -0,0 +1,32 @@
+#include "MantidQtWidgets/Common/ProgressableView.h"
+
+namespace MantidQt {
+namespace MantidWidgets {
+
+bool ProgressableView::isPercentageIndicator() const {
+ return m_style == Style::PERCENTAGE;
+}
+
+void ProgressableView::setProgressRange(int min, int max) {
+ // Cache values for a percentage-style progress bar i.e. where both are not
+ // zero
+ if (min != 0 || max != 0) {
+ m_min = min;
+ m_max = max;
+ }
+}
+
+void ProgressableView::setAsPercentageIndicator() {
+ m_style = Style::PERCENTAGE;
+ setProgressRange(m_min, m_max);
+}
+
+void ProgressableView::setAsEndlessIndicator() {
+ m_style = Style::ENDLESS;
+ // To get QProgressBar to display as an endless progress indicator, we need
+ // to set start=end=0 in the derived view class
+ if (m_style == Style::ENDLESS)
+ setProgressRange(0, 0);
+}
+} // namespace MantidWidgets
+} // namepsace MantidQt
diff --git a/qt/widgets/common/test/ProgressableViewTest.h b/qt/widgets/common/test/ProgressableViewTest.h
new file mode 100644
index 00000000000..7cc0082654f
--- /dev/null
+++ b/qt/widgets/common/test/ProgressableViewTest.h
@@ -0,0 +1,84 @@
+#ifndef MANTID_MANTIDWIDGETS_PROGRESSABLEVIEWTEST_H
+#define MANTID_MANTIDWIDGETS_PROGRESSABLEVIEWTEST_H
+
+#include "MantidQtWidgets/Common/ProgressableView.h"
+#include <cxxtest/TestSuite.h>
+
+using namespace MantidQt::MantidWidgets;
+
+//=====================================================================================
+// Functional tests
+//=====================================================================================
+class ProgressableViewTest : 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 ProgressableViewTest *createSuite() {
+ return new ProgressableViewTest();
+ }
+ static void destroySuite(ProgressableViewTest *suite) { delete suite; }
+
+ ProgressableViewTest() {}
+
+ void testSetProgressRange() {
+ int min = 5;
+ int max = 18;
+ m_progress.setProgressRange(min, max);
+ m_progress.assertRange(min, max);
+ }
+
+ void testSetProgressRangeBothZero() {
+ // Set a non-zero range first
+ int min = 5;
+ int max = 18;
+ m_progress.setProgressRange(min, max);
+ // Now set start=end=0
+ m_progress.setProgressRange(0, 0);
+ // A 0-0 range is a special case and should not be cached, so we should
+ // still have the original range
+ m_progress.assertRange(min, max);
+ }
+
+ void testSetProgressRangeZeroLength() {
+ int min = 7;
+ int max = 7;
+ m_progress.setProgressRange(min, max);
+ m_progress.assertRange(min, max);
+ }
+
+ void testSetPercentageIndicator() {
+ m_progress.setAsPercentageIndicator();
+ m_progress.assertStyle(ProgressableView::Style::PERCENTAGE);
+ }
+
+ void testSetEndlessIndicator() {
+ m_progress.setAsEndlessIndicator();
+ m_progress.assertStyle(ProgressableView::Style::ENDLESS);
+ }
+
+ void testRangeNotLostChangeStyle() {
+ int min = 5;
+ int max = 18;
+ m_progress.setProgressRange(min, max);
+ m_progress.setAsEndlessIndicator();
+ m_progress.assertRange(min, max);
+ }
+
+private:
+ // Inner class :: fake progressable view
+ class ProgressBar : public ProgressableView {
+ public:
+ void setProgress(int) override {}
+ void clearProgress() override {}
+ void assertRange(int min, int max) const {
+ TS_ASSERT_EQUALS(m_min, min);
+ TS_ASSERT_EQUALS(m_max, max);
+ }
+ void assertStyle(Style style) const { TS_ASSERT_EQUALS(m_style, style); }
+ };
+
+ ProgressBar m_progress;
+};
+
+#endif /*MANTID_MANTIDWIDGETS_PROGRESSABLEVIEWTEST_H */
--
GitLab