From 5e2ba22d8bb09a6c1f8045fd433098dfe5cd1f63 Mon Sep 17 00:00:00 2001
From: Robert Applin <40830825+robertapplin@users.noreply.github.com>
Date: Wed, 17 Oct 2018 09:50:24 +0100
Subject: [PATCH] Refs #23463. Finish IndirectFitPlotModel tests and tidy

---
 .../test/IndirectFitDataCreationHelperTest.h  |   2 +
 .../IndirectFitDataCreationHelper.h           |  34 +----
 .../Indirect/IndirectFitPlotModel.cpp         |   2 +-
 .../Indirect/test/IndirectFitDataTest.h       |   2 +
 .../Indirect/test/IndirectFitOutputTest.h     |  43 +++----
 .../Indirect/test/IndirectFitPlotModelTest.h  | 118 ++++++++++++++----
 .../Indirect/test/IndirectFittingModelTest.h  |  11 +-
 7 files changed, 121 insertions(+), 91 deletions(-)

diff --git a/Framework/Algorithms/test/IndirectFitDataCreationHelperTest.h b/Framework/Algorithms/test/IndirectFitDataCreationHelperTest.h
index bf52c1ba5b4..2228fff9c6d 100644
--- a/Framework/Algorithms/test/IndirectFitDataCreationHelperTest.h
+++ b/Framework/Algorithms/test/IndirectFitDataCreationHelperTest.h
@@ -61,6 +61,8 @@ public:
     delete suite;
   }
 
+  void tearDown() override { AnalysisDataService::Instance().clear(); }
+
   void
   test_that_createWorkspace_returns_a_workspace_with_the_number_of_spectra_specified() {
     auto const workspace = createWorkspace(10);
diff --git a/Framework/TestHelpers/inc/MantidTestHelpers/IndirectFitDataCreationHelper.h b/Framework/TestHelpers/inc/MantidTestHelpers/IndirectFitDataCreationHelper.h
index f1d50a61b31..961b4f5a339 100644
--- a/Framework/TestHelpers/inc/MantidTestHelpers/IndirectFitDataCreationHelper.h
+++ b/Framework/TestHelpers/inc/MantidTestHelpers/IndirectFitDataCreationHelper.h
@@ -31,7 +31,8 @@ setWorkspaceProperties(Mantid::API::MatrixWorkspace_sptr workspace,
 Mantid::API::MatrixWorkspace_sptr
 createWorkspaceWithInstrument(int const &xLength, int const &yLength);
 
-/// Simple struct to set up the ADS with the configuration required
+/// Simple struct used to access features of the ADS
+/// No destructor so ensure you tearDown the ADS
 struct SetUpADSWithWorkspace {
 
   template <typename T>
@@ -56,37 +57,6 @@ struct SetUpADSWithWorkspace {
     return boost::dynamic_pointer_cast<Mantid::API::MatrixWorkspace>(
         Mantid::API::AnalysisDataService::Instance().retrieve(workspaceName));
   }
-
-  ~SetUpADSWithWorkspace() {
-    Mantid::API::AnalysisDataService::Instance().clear();
-  }
-};
-
-/// This struct gives the option of having an ADS instance with no destructor
-struct SetUpADSWithNoDestructor {
-
-  template <typename T>
-  SetUpADSWithNoDestructor(std::string const &inputWSName, T const &workspace) {
-    Mantid::API::AnalysisDataService::Instance().addOrReplace(inputWSName,
-                                                              workspace);
-  }
-
-  template <typename T>
-  void addOrReplace(std::string const &workspaceName, T const &workspace) {
-    Mantid::API::AnalysisDataService::Instance().addOrReplace(workspaceName,
-                                                              workspace);
-  }
-
-  bool doesExist(std::string const &workspaceName) {
-    return Mantid::API::AnalysisDataService::Instance().doesExist(
-        workspaceName);
-  }
-
-  Mantid::API::MatrixWorkspace_sptr
-  retrieveWorkspace(std::string const &workspaceName) {
-    return boost::dynamic_pointer_cast<Mantid::API::MatrixWorkspace>(
-        Mantid::API::AnalysisDataService::Instance().retrieve(workspaceName));
-  }
 };
 
 /// This is used to compare Spectra which is implemented as a boost::variant
diff --git a/qt/scientific_interfaces/Indirect/IndirectFitPlotModel.cpp b/qt/scientific_interfaces/Indirect/IndirectFitPlotModel.cpp
index d26fb27a14d..634c668f122 100644
--- a/qt/scientific_interfaces/Indirect/IndirectFitPlotModel.cpp
+++ b/qt/scientific_interfaces/Indirect/IndirectFitPlotModel.cpp
@@ -17,7 +17,7 @@
 namespace {
 using namespace Mantid::API;
 
-// The name of the conjoined input and guess name -- required for
+// The name of the conjoined input and guess workspaces -- required for
 // creating an external guess plot.
 const std::string INPUT_AND_GUESS_NAME = "__QENSInputAndGuess";
 
diff --git a/qt/scientific_interfaces/Indirect/test/IndirectFitDataTest.h b/qt/scientific_interfaces/Indirect/test/IndirectFitDataTest.h
index 11a6383bc9e..68348bf5035 100644
--- a/qt/scientific_interfaces/Indirect/test/IndirectFitDataTest.h
+++ b/qt/scientific_interfaces/Indirect/test/IndirectFitDataTest.h
@@ -36,6 +36,8 @@ public:
 
   static void destroySuite(IndirectFitDataTest *suite) { delete suite; }
 
+  void tearDown() override { AnalysisDataService::Instance().clear(); }
+
   void test_data_is_instantiated() {
     auto const workspace = createWorkspace(1);
     Spectra const spec =
diff --git a/qt/scientific_interfaces/Indirect/test/IndirectFitOutputTest.h b/qt/scientific_interfaces/Indirect/test/IndirectFitOutputTest.h
index 1d26559a2cb..2e53b496b19 100644
--- a/qt/scientific_interfaces/Indirect/test/IndirectFitOutputTest.h
+++ b/qt/scientific_interfaces/Indirect/test/IndirectFitOutputTest.h
@@ -80,6 +80,14 @@ WorkspaceGroup_sptr getPopulatedGroup(std::size_t const &size) {
   return group;
 }
 
+/// Store workspaces in ADS and won't destruct the ADS when leaving scope
+void storeWorkspacesInADS(WorkspaceGroup_sptr group,
+                          ITableWorkspace_sptr table) {
+  SetUpADSWithWorkspace ads("ResultGroup", group);
+  ads.addOrReplace("ResultWorkspaces", group);
+  ads.addOrReplace("ParameterTable", table);
+}
+
 std::unique_ptr<IndirectFitOutput>
 createFitOutput(WorkspaceGroup_sptr resultGroup,
                 ITableWorkspace_sptr parameterTable,
@@ -89,41 +97,17 @@ createFitOutput(WorkspaceGroup_sptr resultGroup,
       resultGroup, parameterTable, resultWorkspace, fitData, spectrum);
 }
 
-/// Used to create fit output data where the ads is destructed after leaving the
-/// function scope
-std::unique_ptr<IndirectFitOutput> getDestructedFitOutputData() {
-  auto const group = getPopulatedGroup(2);
-  auto const table = getPopulatedTable(2);
-  IndirectFitData *data = new IndirectFitData(getIndirectFitData(5));
-
-  SetUpADSWithWorkspace ads("ResultGroup", group);
-  ads.addOrReplace("ResultWorkspaces", group);
-  ads.addOrReplace("ParameterTable", table);
-
-  return createFitOutput(group, table, group, data, 0);
-}
-
 /// This will return fit output with workspaces still stored in the ADS
 std::unique_ptr<IndirectFitOutput> getFitOutputData() {
   auto const group = getPopulatedGroup(2);
   auto const table = getPopulatedTable(2);
   IndirectFitData *data = new IndirectFitData(getIndirectFitData(5));
 
-  SetUpADSWithNoDestructor ads("ResultGroup", group);
-  ads.addOrReplace("ResultWorkspaces", group);
-  ads.addOrReplace("ParameterTable", table);
+  storeWorkspacesInADS(group, table);
 
   return createFitOutput(group, table, group, data, 0);
 }
 
-/// Store workspaces in ADS and won't destruct the ADS when leaving scope
-void storeWorkspacesInADS(WorkspaceGroup_sptr group,
-                          ITableWorkspace_sptr table) {
-  SetUpADSWithNoDestructor ads("ResultGroup", group);
-  ads.addOrReplace("ResultWorkspaces", group);
-  ads.addOrReplace("ParameterTable", table);
-}
-
 std::unordered_map<std::string, std::string>
 getNewParameterNames(std::vector<std::string> const &currentNames) {
   std::unordered_map<std::string, std::string> newParameterNames;
@@ -267,9 +251,12 @@ public:
 
   void
   test_that_getResultParameterNames_returns_an_empty_vector_if_the_result_workspace_cannot_be_found() {
-    /// The fact that the result workspace has been removed from the ADS is used
-    /// here. This means the result workspace won't be available any longer.
-    auto const output = getDestructedFitOutputData();
+    /// The fact that the result workspace has been removed from the ADS means
+    /// the parameter names won't be available any longer.
+    auto const output = getFitOutputData();
+
+    AnalysisDataService::Instance().clear();
+
     TS_ASSERT(output->getResultParameterNames().empty());
   }
 
diff --git a/qt/scientific_interfaces/Indirect/test/IndirectFitPlotModelTest.h b/qt/scientific_interfaces/Indirect/test/IndirectFitPlotModelTest.h
index 57c55c1ab93..a5963077023 100644
--- a/qt/scientific_interfaces/Indirect/test/IndirectFitPlotModelTest.h
+++ b/qt/scientific_interfaces/Indirect/test/IndirectFitPlotModelTest.h
@@ -21,13 +21,16 @@ using ConvolutionFitSequential =
 
 namespace {
 
+/// The name of the conjoined input and guess workspaces
+std::string const INPUT_AND_GUESS_NAME = "__QENSInputAndGuess";
+
 std::string getFittingFunctionString(std::string const &workspaceName) {
   return "name=LinearBackground,A0=0,A1=0,ties=(A0=0.000000,A1=0.0);"
          "(composite=Convolution,FixResolution=true,NumDeriv=true;"
          "name=Resolution,Workspace=" +
          workspaceName +
          ",WorkspaceIndex=0;((composite=ProductFunction,NumDeriv="
-         "false;name=Lorentzian,Amplitude=1,PeakCentre=0,FWHM=0."
+         "false;name=Lorentzian,Amplitude=1,PeakCentre=1,FWHM=0."
          "0175)))";
 }
 
@@ -54,19 +57,23 @@ private:
 };
 
 void setFittingFunction(IndirectFittingModel *model,
-                        std::string const &functionString) {
-  model->setFitFunction(getFunction(functionString));
+                        std::string const &functionString,
+                        bool setFitFunction) {
+  if (setFitFunction)
+    model->setFitFunction(getFunction(functionString));
 }
 
 IndirectFittingModel *getEmptyDummyModel() { return new DummyModel(); }
 
 IndirectFittingModel *
 createModelWithSingleWorkspace(std::string const &workspaceName,
-                               int const &numberOfSpectra) {
+                               int const &numberOfSpectra,
+                               bool setFitFunction) {
   auto model = getEmptyDummyModel();
-  SetUpADSWithNoDestructor ads(workspaceName, createWorkspace(numberOfSpectra));
+  SetUpADSWithWorkspace ads(workspaceName, createWorkspace(numberOfSpectra));
   model->addWorkspace(workspaceName);
-  setFittingFunction(model, getFittingFunctionString(workspaceName));
+  setFittingFunction(model, getFittingFunctionString(workspaceName),
+                     setFitFunction);
   return model;
 }
 
@@ -80,17 +87,17 @@ template <typename Name, typename... Names>
 void addWorkspacesToModel(IndirectFittingModel *model,
                           int const &numberOfSpectra, Name const &workspaceName,
                           Names const &... workspaceNames) {
-  SetUpADSWithNoDestructor ads(workspaceName, createWorkspace(numberOfSpectra));
+  SetUpADSWithWorkspace ads(workspaceName, createWorkspace(numberOfSpectra));
   model->addWorkspace(workspaceName);
   addWorkspacesToModel(model, numberOfSpectra, workspaceNames...);
 }
 
 template <typename Name, typename... Names>
-IndirectFittingModel *
-createModelWithMultipleWorkspaces(int const &numberOfSpectra,
-                                  Name const &workspaceName,
-                                  Names const &... workspaceNames) {
-  auto model = createModelWithSingleWorkspace(workspaceName, numberOfSpectra);
+IndirectFittingModel *createModelWithMultipleWorkspaces(
+    int const &numberOfSpectra, bool setFitFunction, Name const &workspaceName,
+    Names const &... workspaceNames) {
+  auto model = createModelWithSingleWorkspace(workspaceName, numberOfSpectra,
+                                              setFitFunction);
   addWorkspacesToModel(model, numberOfSpectra, workspaceNames...);
   return model;
 }
@@ -98,8 +105,8 @@ createModelWithMultipleWorkspaces(int const &numberOfSpectra,
 IndirectFittingModel *createModelWithSingleInstrumentWorkspace(
     std::string const &workspaceName, int const &xLength, int const &yLength) {
   auto model = getEmptyDummyModel();
-  SetUpADSWithNoDestructor ads(workspaceName,
-                               createWorkspaceWithInstrument(xLength, yLength));
+  SetUpADSWithWorkspace ads(workspaceName,
+                            createWorkspaceWithInstrument(xLength, yLength));
   model->addWorkspace(workspaceName);
   return model;
 }
@@ -125,7 +132,7 @@ IAlgorithm_sptr setupFitAlgorithm(MatrixWorkspace_sptr workspace,
 IAlgorithm_sptr getSetupFitAlgorithm(IndirectFittingModel *model,
                                      MatrixWorkspace_sptr workspace,
                                      std::string const &workspaceName) {
-  setFittingFunction(model, getFittingFunctionString(workspaceName));
+  setFittingFunction(model, getFittingFunctionString(workspaceName), true);
   auto alg =
       setupFitAlgorithm(workspace, getFittingFunctionString(workspaceName));
   return alg;
@@ -142,16 +149,15 @@ IAlgorithm_sptr getExecutedFitAlgorithm(IndirectFittingModel *model,
 IndirectFittingModel *getModelWithFitOutputData() {
   auto model = createModelWithSingleInstrumentWorkspace("__ConvFit", 6, 5);
   auto const modelWorkspace = model->getWorkspace(0);
-  SetUpADSWithNoDestructor ads("__ConvFit", modelWorkspace);
 
   auto const alg = getExecutedFitAlgorithm(model, modelWorkspace, "__ConvFit");
   model->addOutput(alg);
   return model;
 }
 
-IndirectFitPlotModel getFitPlotModel() {
-  return IndirectFitPlotModel(
-      createModelWithMultipleWorkspaces(10, "Workspace1", "Workspace2"));
+IndirectFitPlotModel getFitPlotModel(bool setFitFunction = true) {
+  return IndirectFitPlotModel(createModelWithMultipleWorkspaces(
+      10, setFitFunction, "Workspace1", "Workspace2"));
 }
 
 IndirectFitPlotModel getFitPlotModelWithFitData() {
@@ -225,7 +231,7 @@ public:
 
     auto const resultWorkspace = model.appendGuessToInput(guess);
 
-    TS_ASSERT(AnalysisDataService::Instance().doesExist("__QENSInputAndGuess"));
+    TS_ASSERT(AnalysisDataService::Instance().doesExist(INPUT_AND_GUESS_NAME));
     TS_ASSERT_EQUALS(resultWorkspace->getAxis(1)->label(0), "Sample");
     TS_ASSERT_EQUALS(resultWorkspace->getAxis(1)->label(1), "Guess");
     /// Only two spectra because the guessWorkspace will only ever have one
@@ -328,7 +334,7 @@ public:
   void
   test_that_getFirstPeakCentre_returns_the_value_of_the_first_PeakCentre_in_the_fitting_function() {
     auto const model = getFitPlotModel();
-    TS_ASSERT_EQUALS(model.getFirstPeakCentre(), 0.0);
+    TS_ASSERT_EQUALS(model.getFirstPeakCentre(), 1.0);
   }
 
   void
@@ -336,6 +342,76 @@ public:
     auto const model = getFitPlotModel();
     TS_ASSERT_EQUALS(model.getFirstBackgroundLevel(), 0.0);
   }
+
+  void test_that_calculateHWHMMaximum_returns_the_value_expected() {
+    auto const model = getFitPlotModel();
+
+    auto const hwhm = model.getFirstHWHM();
+    auto const peakCentre = model.getFirstPeakCentre().get_value_or(0.);
+
+    auto const minimum = peakCentre + *hwhm;
+    TS_ASSERT_EQUALS(model.calculateHWHMMaximum(minimum), 0.99125);
+  }
+
+  void test_that_calculateHWHMMinimum_returns_the_value_expected() {
+    auto const model = getFitPlotModel();
+
+    auto const hwhm = model.getFirstHWHM();
+    auto const peakCentre = model.getFirstPeakCentre().get_value_or(0.);
+
+    auto const maximum = peakCentre - *hwhm;
+    TS_ASSERT_EQUALS(model.calculateHWHMMinimum(maximum), 1.00875);
+  }
+
+  void
+  test_that_canCalculateGuess_returns_false_when_there_is_no_fitting_function() {
+    auto const model = getFitPlotModel(false);
+    TS_ASSERT(!model.canCalculateGuess());
+  }
+
+  void
+  test_that_canCalculateGuess_returns_true_when_there_is_a_fitting_function_and_a_model_with_a_workspace() {
+    auto const model = getFitPlotModel();
+    TS_ASSERT(model.canCalculateGuess());
+  }
+
+  void
+  test_that_setFWHM_will_change_the_value_of_the_FWHM_in_the_fitting_function() {
+    auto model = getFitPlotModel();
+
+    auto const fwhm = 1.1;
+    model.setFWHM(fwhm);
+
+    TS_ASSERT_EQUALS(model.getFirstHWHM(), fwhm / 2);
+  }
+
+  void
+  test_that_setBackground_will_change_the_value_of_A0_in_the_fitting_function() {
+    auto model = getFitPlotModel();
+
+    auto const background = 0.12;
+    model.setBackground(background);
+
+    TS_ASSERT_EQUALS(model.getFirstBackgroundLevel(), background);
+  }
+
+  void
+  test_that_deleteExternalGuessWorkspace_removes_the_guess_workspace_from_the_ADS() {
+    auto model = getFitPlotModel();
+
+    auto const guess = model.getGuessWorkspace();
+    (void)model.appendGuessToInput(guess);
+
+    TS_ASSERT(AnalysisDataService::Instance().doesExist(INPUT_AND_GUESS_NAME));
+    model.deleteExternalGuessWorkspace();
+    TS_ASSERT(!AnalysisDataService::Instance().doesExist(INPUT_AND_GUESS_NAME));
+  }
+
+  void
+  test_that_deleteExternalGuessWorkspace_does_not_throw_if_the_guess_workspace_does_not_exist() {
+    auto model = getFitPlotModel();
+    TS_ASSERT_THROWS_NOTHING(model.deleteExternalGuessWorkspace());
+  }
 };
 
 #endif
diff --git a/qt/scientific_interfaces/Indirect/test/IndirectFittingModelTest.h b/qt/scientific_interfaces/Indirect/test/IndirectFittingModelTest.h
index 88f0af15bbf..b4bb2f333fc 100644
--- a/qt/scientific_interfaces/Indirect/test/IndirectFittingModelTest.h
+++ b/qt/scientific_interfaces/Indirect/test/IndirectFittingModelTest.h
@@ -140,7 +140,6 @@ IAlgorithm_sptr getExecutedFitAlgorithm(std::unique_ptr<DummyModel> &model,
 std::unique_ptr<DummyModel> getModelWithFitOutputData() {
   auto model = createModelWithSingleInstrumentWorkspace("__ConvFit", 6, 5);
   auto const modelWorkspace = model->getWorkspace(0);
-  SetUpADSWithWorkspace ads("__ConvFit", modelWorkspace);
 
   auto const alg = getExecutedFitAlgorithm(model, modelWorkspace, "__ConvFit");
   model->addOutput(alg);
@@ -160,6 +159,8 @@ public:
 
   static void destroySuite(IndirectFittingModelTest *suite) { delete suite; }
 
+  void tearDown() override { AnalysisDataService::Instance().clear(); }
+
   void test_model_is_instantiated_correctly() {
     auto model = createModelWithSingleWorkspace("WorkspaceName", 3);
 
@@ -399,7 +400,6 @@ public:
   void test_that_ConvolutionSequentialFit_algorithm_initializes() {
     auto model = createModelWithSingleInstrumentWorkspace("Name", 6, 5);
     auto const modelWorkspace = model->getWorkspace(0);
-    SetUpADSWithWorkspace ads("Name", modelWorkspace);
 
     auto const alg = getSetupFitAlgorithm(model, modelWorkspace, "Name");
 
@@ -409,7 +409,6 @@ public:
   void test_that_ConvolutionSequentialFit_algorithm_executes_without_error() {
     auto model = createModelWithSingleInstrumentWorkspace("Name", 6, 5);
     auto const modelWorkspace = model->getWorkspace(0);
-    SetUpADSWithWorkspace ads("Name", modelWorkspace);
 
     auto const alg = getSetupFitAlgorithm(model, modelWorkspace, "Name");
 
@@ -420,7 +419,6 @@ public:
   void test_that_addOutput_adds_the_output_of_a_fit_into_the_model() {
     auto model = createModelWithSingleInstrumentWorkspace("__ConvFit", 6, 5);
     auto const modelWorkspace = model->getWorkspace(0);
-    SetUpADSWithWorkspace ads("__ConvFit", modelWorkspace);
 
     auto const alg =
         getExecutedFitAlgorithm(model, modelWorkspace, "__ConvFit");
@@ -477,7 +475,6 @@ public:
   void test_isInvalidFunction_returns_none_if_the_activeFunction_is_valid() {
     auto model = createModelWithSingleInstrumentWorkspace("Name", 6, 5);
     auto const modelWorkspace = model->getWorkspace(0);
-    SetUpADSWithWorkspace ads("Name", modelWorkspace);
 
     (void)getSetupFitAlgorithm(model, modelWorkspace, "Name");
 
@@ -513,7 +510,6 @@ public:
   test_that_getFitParameterNames_returns_a_vector_of_fit_parameters_if_the_fitOutput_contains_parameters() {
     auto model = createModelWithSingleInstrumentWorkspace("__ConvFit", 6, 5);
     auto const modelWorkspace = model->getWorkspace(0);
-    SetUpADSWithWorkspace ads("__ConvFit", modelWorkspace);
 
     auto const alg =
         getExecutedFitAlgorithm(model, modelWorkspace, "__ConvFit");
@@ -619,7 +615,6 @@ public:
   test_that_setDefaultParameterValue_will_set_the_value_of_the_provided_parameter() {
     auto model = createModelWithSingleWorkspace("Name", 5);
     auto const modelWorkspace = model->getWorkspace(0);
-    SetUpADSWithWorkspace ads("Name", modelWorkspace);
 
     (void)getSetupFitAlgorithm(model, modelWorkspace, "Name");
     model->setDefaultParameterValue("Amplitude", 1.5, 0);
@@ -638,7 +633,6 @@ public:
   test_that_getParameterValues_returns_the_default_parameters_if_there_are_no_fit_parameters() {
     auto model = createModelWithSingleInstrumentWorkspace("__ConvFit", 6, 5);
     auto const modelWorkspace = model->getWorkspace(0);
-    SetUpADSWithWorkspace ads("__ConvFit", modelWorkspace);
 
     (void)getSetupFitAlgorithm(model, modelWorkspace, "__ConvFit");
     model->setDefaultParameterValue("Amplitude", 1.5, 0);
@@ -660,7 +654,6 @@ public:
   void test_getFitParameters_returns_an_empty_map_when_there_is_no_fitOutput() {
     auto model = createModelWithSingleInstrumentWorkspace("__ConvFit", 6, 5);
     auto const modelWorkspace = model->getWorkspace(0);
-    SetUpADSWithWorkspace ads("__ConvFit", modelWorkspace);
 
     (void)getSetupFitAlgorithm(model, modelWorkspace, "__ConvFit");
 
-- 
GitLab