From 6d826b6d332b754dc31d9b4679c925821dfa11cd Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Thu, 21 Aug 2025 09:55:27 +0200 Subject: [PATCH 1/4] #5464 - Add a failing test (hits an assert in debug) --- src/measure/test/OSMeasure_GTest.cpp | 76 ++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/measure/test/OSMeasure_GTest.cpp b/src/measure/test/OSMeasure_GTest.cpp index ca1f2411136..30d96e168eb 100644 --- a/src/measure/test/OSMeasure_GTest.cpp +++ b/src/measure/test/OSMeasure_GTest.cpp @@ -457,3 +457,79 @@ TEST_F(MeasureFixture, UserScript_TestModelUserScriptDomain) { EXPECT_EQ("Integer User argument 'int_arg' has a value '-3' that is not in the domain [0, 2147483647].", result.stepErrors()[0]); EXPECT_EQ(0u, result.stepWarnings().size()); } + +// Test for #5464 - Shouldn't throw an error when a separator is used in the arguments +class ModelMeasureWithSeparator : public ModelMeasure +{ + public: + virtual std::string name() const override { + return "ModelMeasureWithSeparator"; + } + + virtual std::vector arguments(const Model& /*model*/) const override { + std::vector result; + + OSArgument arg = OSArgument::makeDoubleArgument("double_arg", true); + arg.setMaxValue(10.0); + result.push_back(arg); + + arg = OSArgument::makeSeparatorArgument("separator"); + result.push_back(arg); + + arg = OSArgument::makeIntegerArgument("int_arg", true); + arg.setMinValue(0); + result.push_back(arg); + + return result; + } + + // remove all spaces and add a new one + virtual bool run(Model& model, OSRunner& runner, const std::map& user_arguments) const override { + ModelMeasure::run(model, runner, user_arguments); // initializes runner + + return runner.validateUserArguments(arguments(model), user_arguments); + } +}; + +TEST_F(MeasureFixture, ModelMeasureWithSeparator) { + ModelMeasureWithSeparator measure; + EXPECT_EQ("ModelMeasureWithSeparator", measure.name()); + + Model model; + + std::vector steps; + steps.push_back(MeasureStep("dummy")); + + WorkflowJSON workflow; + workflow.setWorkflowSteps(steps); + + TestOSRunner runner(workflow); + OSArgumentVector arguments = measure.arguments(model); + std::map argumentMap = convertOSArgumentVectorToMap(arguments); + ASSERT_EQ(3, argumentMap.size()); + + OSArgument& double_arg = argumentMap["double_arg"]; + OSArgument& int_arg = argumentMap["int_arg"]; + + // call with a good value + double_arg.setValue(-1.0); + int_arg.setValue(1.0); + EXPECT_TRUE(measure.run(model, runner, argumentMap)); + WorkflowStepResult result = runner.result(); + ASSERT_TRUE(result.stepResult()); + EXPECT_EQ(StepResult::Success, result.stepResult()->value()); + EXPECT_EQ(0u, result.stepErrors().size()); + EXPECT_EQ(0u, result.stepWarnings().size()); + + // Out of bound value for int_arg + runner.reset(); + double_arg.setValue(1.0); + int_arg.setValue(-3); + EXPECT_FALSE(measure.run(model, runner, argumentMap)); + result = runner.result(); + ASSERT_TRUE(result.stepResult()); + EXPECT_EQ(StepResult::Fail, result.stepResult()->value()); + ASSERT_EQ(1u, result.stepErrors().size()); + EXPECT_EQ("Integer User argument 'int_arg' has a value '-3' that is not in the domain [0, 2147483647].", result.stepErrors()[0]); + EXPECT_EQ(0u, result.stepWarnings().size()); +} From a53f31c74f42892f3a0374e0295c4597fec9d887 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Thu, 21 Aug 2025 10:13:14 +0200 Subject: [PATCH 2/4] Make it fail in release too by capturing the assert via a StringStreamLogSink /media/DataExt4/Software/Others/OpenStudio3/src/measure/test/OSMeasure_GTest.cpp:523: Failure Expected equality of these values: 0 sink.logMessages().size() Which is: 1 [BOOST_ASSERT] <2> Assertion false failed on line 585 of bool openstudio::measure::OSRunner::validateUserArguments(const std::vector&, const std::map, openstudio::measure::OSArgument>&) in file /media/DataExt4/Software/Others/OpenStudio3/src/measure/OSRunner.cpp. --- src/measure/test/OSMeasure_GTest.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/measure/test/OSMeasure_GTest.cpp b/src/measure/test/OSMeasure_GTest.cpp index 30d96e168eb..4267c31d70f 100644 --- a/src/measure/test/OSMeasure_GTest.cpp +++ b/src/measure/test/OSMeasure_GTest.cpp @@ -21,6 +21,7 @@ #include #include "../../utilities/core/Finder.hpp" +#include "../../utilities/core/StringStreamLogSink.hpp" #include "../../utilities/filetypes/WorkflowJSON.hpp" #include "../../utilities/filetypes/WorkflowStep.hpp" @@ -511,10 +512,17 @@ TEST_F(MeasureFixture, ModelMeasureWithSeparator) { OSArgument& double_arg = argumentMap["double_arg"]; OSArgument& int_arg = argumentMap["int_arg"]; + StringStreamLogSink sink; + sink.setLogLevel(Fatal); + // call with a good value double_arg.setValue(-1.0); int_arg.setValue(1.0); EXPECT_TRUE(measure.run(model, runner, argumentMap)); + + EXPECT_EQ(0, sink.logMessages().size()) << sink.string(); + sink.resetStringStream(); + WorkflowStepResult result = runner.result(); ASSERT_TRUE(result.stepResult()); EXPECT_EQ(StepResult::Success, result.stepResult()->value()); @@ -526,6 +534,10 @@ TEST_F(MeasureFixture, ModelMeasureWithSeparator) { double_arg.setValue(1.0); int_arg.setValue(-3); EXPECT_FALSE(measure.run(model, runner, argumentMap)); + + EXPECT_EQ(0, sink.logMessages().size()) << sink.string(); + sink.resetStringStream(); + result = runner.result(); ASSERT_TRUE(result.stepResult()); EXPECT_EQ(StepResult::Fail, result.stepResult()->value()); From b22163c01ce53dcdc94cdc8e8899b1ddeaffecea Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Thu, 21 Aug 2025 10:14:04 +0200 Subject: [PATCH 3/4] Fix #5464 - Skip validating Separator Arguments so won't hit an os_assert later --- src/measure/OSRunner.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/measure/OSRunner.cpp b/src/measure/OSRunner.cpp index 5b7309355af..c2832d657ea 100644 --- a/src/measure/OSRunner.cpp +++ b/src/measure/OSRunner.cpp @@ -409,6 +409,10 @@ namespace measure { bool result = true; std::vector stepValues; for (const OSArgument& script_argument : script_arguments) { + if (script_argument.type().value() == OSArgumentType::Separator) { + // skip separators + continue; + } auto it = user_arguments.find(script_argument.name()); if (it == user_arguments.end()) { // script_argument is not in user_arguments From 9adf6d811d01ffd1eef5cf4ea2f3b729a4177323 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Thu, 21 Aug 2025 10:16:07 +0200 Subject: [PATCH 4/4] Add "Separator" argument type to measure 3.1 schema cf https://github.com/NREL/OpenStudio/issues/5464#issuecomment-3209354052 ``` [openstudio.XMLValidator] <1> xsdValidate.parseFileError: XML error 17.1840: Element 'type': [facet 'enumeration'] The value 'Separator' is not an element of the set {'Boolean', 'Double', 'Integer', 'String', 'Choice', 'Path'}. at /home/julien/Downloads/temp/5464/RubyModelMeasure/measure.xml:17 while processing "Separator" [openstudio.XMLValidator] <1> xsdValidate.parseFileError: XML error 17.1840: Element 'type': [facet 'enumeration'] The value 'Separator' is not an element of the set {'Boolean', 'Double', 'Integer', 'String', 'Choice', 'Path'}. ``` --- src/utilities/xml/resources/bcl/measure_v3.1.xsd | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utilities/xml/resources/bcl/measure_v3.1.xsd b/src/utilities/xml/resources/bcl/measure_v3.1.xsd index f7f9d138cd9..962f0b74717 100644 --- a/src/utilities/xml/resources/bcl/measure_v3.1.xsd +++ b/src/utilities/xml/resources/bcl/measure_v3.1.xsd @@ -87,6 +87,7 @@ +