Some Python algorithm tests are hiding bugs

[jhaigh0]

I have run into some issue which haven't been picked up in tests which should have.

When a PythonAlgorithm is called it runs validateInputs which returns a dictionary of issues used to tell the user which inputs are wrong and why. It seems that when this dictionary is not empty some kind of RuntimeError is raised, opening a message box.

The problem is that in many tests for these algorithms, when testing that inputs are validated correctly the test simply excepts a RuntimeError. This error could then be something like a typo or really any of kind of exception raised by the code and the test will still pass because it does not query this exception. Exceptions like this will often result in crashes.

For example:

• In Mean algorithm input workspace validation #34556 I have refactored the old tests to query the exception and check it is as intended. The old tests were hiding some bugs which the PR fixed and whilst refactoring I discovered another bug in the algorithm code not picked up by the tests.
• In Fix for bad validation in AddSampleLogMultiple #34566 a typo which caused a crash was not picked up by the tests since they were excepting any RuntimeError

Describe the outcome that is desired.
Testing validation of test inputs should not hide all RuntimeError raises and only pass if the exact correct warning is raised.

Describe any solutions you are considering
Me and @thomashampson thought of two approaches.

1. Query the exception (as in Mean algorithm input workspace validation #34556). This is a little clunky and hardcoded; the exception strings should probably be constants which are then imported into the test file so they don't have to be changed in two places.
2. Additionally, it might be good to make a custom exception for these input warnings so it is safer and more obvious in the code. I think it would still be good to test that the correct exception string is present.

Additional context
Some tests are testing in a safer way. For example MatchPeaksTest.py which querys the exception string (although only checks that is an exception raised by validateInputs and not what it is specifically about).

Files looked into

Part1 #35289

• AbinsAdvancedParametersTest.py
• AbinsBasicTest.py
• ApplyDetectorScanEffCorrTest.py
• BinWidthAtXTest.py
• CalculateEfficiencyCorrectionTest.py
• CalculateSampleTransmissionTest.py
• ComputeIncoherentDOSTest.py
• CreateCacheFilenameTest.py
• CylinderPaalmanPingsCorrection2Test.py
• DNSComputeDetEffCorrCoefsTest.py
• DNSFlippingRatioCorrTest.py
• DNSMergeRunsTest.py

Part 2 #35297

• EnggCalibrateFullTest.py *
• EnggCalibrateTest.py *
• EnggEstimateFocussedBackgroundTest.py
• EnggFitPeaksTest.py *
• EnggFitTOFFromPeaksTest.py *
• EnggFocusTest.py
• EnggVanadiumCorrectionsTest.py *
• ExtractMonitorsTest.py
• FindGlobalBMatrixTest.py
• FindGoniometerFromUBTest.py
• FitGaussianTest.py
• GenerateLogbookTest.py
• HB3AAdjustSampleNormTest.py
• IndirectTransmissionTest.py
• LeadPressureCalcTest.py *
• LoadAndMergeTest.py *
• LoadCIFTest.py
• LoadDNSLegacyTest.py
• LoadGundrumOutputTest.py
• LoadLampTest.py
• LoadNMoldyn3AsciiTest.py
• LoadNMoldyn4AsciiDTest.py
• LoadNMoldyn4AsciiTest.py

Part 3 #35306

• MatchPeaksTest.py
• MatchSpectraTest.py
• MeanTest.py
• MedianBinWidthTest.py
• NMoldyn4InterpolationTest.py
• NormaliseSpectraTest.py
• PeakMatchingTest.py *
• PoldiCreatePeaksFromFileTest.py *
• PoldiMergeTest.py
• ReflectometryReductionOneLiveDataTest.py
• RetrieveRunInfoTest.py
• SANSWideAngleCorrectionTest.py
• SaveReflectionsTest.py
• SaveYDATest.py
• SelectNexusFilesByMetadataTest.py
• SetSampleFromLogsTest.py
• SymmetriseTest.py
• TOFTOFCropWorksapceTest.py
• TOFTOFMergeRunsTest.py
• VersuvioPeakPredictionTest.py
• VersuvioPreFitTest.py
• VersuvioResolutionTest.py
• VersuvioThicknessTest.py
• VersuvioTOFFitTest.py

WorksapceAlgorithms Part 4 #35326

• AddSampleLogMultipleTest.py
• ApplyPaalmanPingsCorrectionTest.py *
• BayesStretchTest.py
• D7YIGPositionCalibrationTest.py
• DetectorFloodWeightingTest.py
• DirectILLReductionTest.py
• FindPeakAutomaticTest.py
• FitGaussianPeaksTest.py
• FlatPlatePaalmanPingsCorrectionTest.py
• IndirectReplaceFitResultTest.py
• IndirectSampleChangerTest.py
• IqtFitMultipleTest.py
• IqtFitSequentialTest.py
• ISISIndirectDiffractionReductionTest.py
• ISISIndirectEnergyTransferTest.py
• ISISIndirectEnergyTransferWrapperTest.py

Part 5 #35330

• MatchAndMergeWorkspacesTest.py
• MSDFitTest.py
• OSIRISDiffractionReductionTest.py
• ReflectometryILL_commonTest.py
• ResNorm2Test.py
• SANSDarkRunBackgroundCorrectionTest.py
• SANSFitShiftScaleTest.py
• SANSILLAutoProcessTest.py
• SANSILLMultiProcessTest.py
• SANSILLReduction2Test.py
• SANSStitchTest.py *
• SaveVulcanGSSTest.py *
• SimpleShapeDiscussInelasticTest.py
• SimpleShapeMonteCarloAbsorptionTest.py
• SimulatedDensityOfStatesTest.py
• TimeSliceTest.py
• TOSCABankCorrectionTest.py
• TotScateCalculateSelfScatteringTest.py

[jhaigh0]

To add:
assertRaisesRegex seems like a good function to query the exception message very cleanly. Here's an example from IndexPeaksTest.py

    def test_exec_throws_error_saving_mod_info_when_no_mod_vector_supplied(self):
        with self.assertRaisesRegex(RuntimeError, "SaveModulationInfo"):
            # testing zero (default) modVec with maxOrder=1
            IndexPeaks(
                PeaksWorkspace="test",
                Tolerance=0.12,
                RoundHKLs=False,
                SaveModulationInfo=True,
                MaxOrder=1,
            )